Add start_FABulator command to be able to run FABulator from the FABulous shell

[JakobTernes]

The title pretty much sais it all...
I added the command in the FABulous2.0-development branch, is that correct?
Let me know if anything here should be changed.
The command basically just runs FABulator in a new process.

Before this is merged, I want to add an option to enable/disable logging to the start_FABulator command, as right now the output of FABulator is logged to the FABulous shell (Which can be very annoying if not intended).
I've pretty much got it all running locally already, but I'm not quite happy with how logging it enabled/disabled.
Ideally, you'd simpy be able to pass an additional flag such as -enable_logging, but I'm not sure how to do this in a clean way. Any suggestions are greatly appreciated!
In my quick and dirty solution, I basically just passed an optional argument to start_FABulator and checked if that argument (if it exists) == "-enable_logging".

[KelvinChung2000]

We currently have the "-v" flag for verbose output. argparse allows you to do counts. Something like "-vv" can be set to toggle FABulator debug, and standard "-v" will only do a debug log. Then, you can keep the command clean.

[JakobTernes]

Would you then have to restart the shell with -vv to get FABulator debug info?
I'd think something like that for the start_FABulator command itself would be nice, so if something goes wrong, like if it doesn't start, you could just run start_FABulator -v to get some debug info.

[JakobTernes]

Btw, should it be run_FABulator or start_FABulator?
I'm probably overthinking it

[KelvinChung2000]

Maybe use run_FABulator since it is more consistent. If it does not start or some error raised then it should raise an error in the shell and show what causing the problem. What other information does the debug log include?

…

________________________________ From: Jakob Ternes ***@***.***> Sent: Saturday, August 17, 2024 1:09:20 PM To: FPGA-Research-Manchester/FABulous ***@***.***> Cc: King Chung ***@***.***>; Review requested ***@***.***> Subject: Re: [FPGA-Research-Manchester/FABulous] Add start_FABulator command to be able to run FABulator from the FABulous shell (PR #227) Btw, should it be run_FABulator or start_FABulator? I'm probably overthinking it — Reply to this email directly, view it on GitHub [github.com]<https://urldefense.com/v3/__https://github.com/FPGA-Research-Manchester/FABulous/pull/227*issuecomment-2294841425__;Iw!!PDiH4ENfjr2_Jw!AQe_naAPBQ5jg1Hldn4MWhHlODCySCrCXsBx0o_gNFnbTVqQRZZkx2lQKBJmLPfbvQUrkhHSCYTg5pIaEwnMihbxWGbYzOj6$>, or unsubscribe [github.com]<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AQF2K6PJXLAXD53LU3NQWL3ZR44PBAVCNFSM6AAAAABMUTEVYSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJUHA2DCNBSGU__;!!PDiH4ENfjr2_Jw!AQe_naAPBQ5jg1Hldn4MWhHlODCySCrCXsBx0o_gNFnbTVqQRZZkx2lQKBJmLPfbvQUrkhHSCYTg5pIaEwnMihbxWGvZKyqk$>. You are receiving this because your review was requested.Message ID: ***@***.***>

[JakobTernes]

With the output of FABulator being logged to the FABulous shell, basically all sorts of user actions in FABulator are logged. Also the output of the startup procedure is logged when FABulator is started which is just a bunch of unnecessary text.

Is this is similar to the other cases in which the -v option is used? Then I think this is the cleanest option. It should be documented though (I will do that), so users don't have to dig deep just to find the option to see FABulator output in the shell.
For me it basically just comes down to whats more convienient from a user standpoint.

[KelvinChung2000]

The verbose is basically for anything that is not necessary for users. The better practice probably be a debug output that only output things that are important for debug, then one level for more detail debug and so on. But we have not done that anyway.

[JakobTernes]

I added the verbosity option and had to change the argument from store_true to count to be able to distinguish between -v and -vv. I kept the name, since when I changed it to run_FABulator, it required the fabric to be loaded, which should not be required for this command.
I saw this is due to some check in precmd which automatically requires the fabric to be loaded, whenever a command with 'run' in it is executed. I also observed weird behavior with this: Even when a command doesn't exist (for example 'runrunrun'), as long as it has 'run' in it, the 'fabric not loaded' message is printed instead of the 'unknown command' message. Is this intended behavior? (its also not big deal, so...)

[KelvinChung2000]

Ideally, we should have that fixed. We can fix it later. I thought FABulator always needed the geometry to work. Is that not the case?

[JakobTernes]

You can start FABulator without having a geometry csv file. If you want to actually see the fabric in FABulator, then of course you'd have to go to File > Open and choose the geometry.csv file.

We could think about also opening the file in FABulator at startup, but FABulator pretty much does that already on its own. It stores the last opened file and you can choose to automatically re-open it at startup

[KelvinChung2000]

When calling the command, start the GUI with the generated geometry.csv of the current project makes the best sense. If you want to start the FABulator and check other fabric geometry, you will likely run the FABulator directly without running the FABulaous CLI.

We can have a toggle to enable GUI at startup, but it is probably not the default behaviour since some machines do not run any GUI.

[IAmMarcelJung]

Thanks for this! Everything looks good and works for me!

[JakobTernes]

When calling the command, start the GUI with the generated geometry.csv of the current project makes the best sense. If you want to start the FABulator and check other fabric geometry, you will likely run the FABulator directly without running the FABulaous CLI.

We can have a toggle to enable GUI at startup, but it is probably not the default behaviour since some machines do not run any GUI.

The way its currently set up, its not possible to pass program arguments at startup. That would require to build and package the application into a jar file and the run the jar file with the program argument.
This is still a work in progress on FABulators side and only supported in the develop branch. If we really want to auto-open the current geometry file at startup there are some things to consider:

• We need a way to determine if the geometry has been generated, and if not, we could check if a fabric is loaded and auto-generate the geometry file
• We need a way to pass the files path to FABulator. Currently there is no way to pass this as a program argument.
  So either we pass the path differently (By setting an environment var for example, but feels a bit hacky). The other option would be to 'properly' build and package FABulator into a jar and then execute that jar with the program argument.

We could also just assume that the user has already build and packaged FABulator into a jar and assume it exists. But since there are two ways of building and running FABulator, and the current one (which doesn't support passing program arguments)
is actually the preferred one (since it is simpler), so I wouldn't want to make that assumption.

Maybe we could wait with this until the develop branch changes have been merged. That could take a while though since there is lots of other stuff going on. Alternatively, we could think about other ways of passing the path as mentioned above

[KelvinChung2000]

It is not something to worry about for now. Having the command enable the GUI accessible from the CLI is all we need. Those auto-start features can happen later. If you are happy with the current state, I will merge this.

[JakobTernes]

Added the link, I think we can merge this if the reviewers are happy as well :)