Update program's help string, implement rudimentary check when user supplied CHECKLOG, update documentation

[Florian-Katerndahl]

• The program now offers three new CL-arguments
  • --debug will print the supplied parameters together with the FORCE_CREDENTIALS environment variable and exit
  • -h|--help will print the program's help and exit
  • -p|--purpose will print the program's purpose (taken from the docs) and exit
• previously, no checks seemed to be implemented, when the user supplied the -l|--logs flag prior to assigning to LPATH
• updated respective parts of documentation and docker tutorial to reflect changes

This also addresses #301

I'm open to changes to this commit.

[ernstste]

Thanks Florian

I think the confusion from #301 comes from an issue in the docs. force-level1-csd never had an -h flag for showing the environment variables. This is done by using force env and I'm not sure how that code example got into the docs. Maybe @davidfrantz knows more.

Do we have a use case for the debug function? Most of the arguments are already printed when simply running the program. I feel this rather adds overhead.

The LPATH variable is not checked before assigment, but along with all the other user input in the code block further down.

[Florian-Katerndahl]

Hey Stephan, well that's my bad than. Could've checked that before writing the commit.

I would argue that the additional check, while very rudimentary, might be useful nonetheless. If you don't specify a path, the next argument automatically gets consumed/assigned to LPATH (I didn't check, if that removes said argument from the option string getopt parses). This means, that an erroneously call such as force-level1-csd -l --debug meta/ level1/ queue.txt aoi.shp only reports a missing log file directory. I argue, in this case it should be reported, that most-likely, an argument got assigned to LPATH. Alternatively, the error message could be updated to reflect this unintended behavior. Personally, I prefer the approach proposed in my PR.

As for the debug flag: The sole reason I added it was because of #301 and the thought, that if such behavior is described in the docs, it should be something that exists. I agree, that there's not really a benefit of having such a flag in it's current implementation.

Cheers, Florian

[davidfrantz]

Thanks Florian!

@ernstste, what do you think? Ingest the change? Additional check do not hurt, right?

[davidfrantz]

@ernstste, where exactly is this line in the docs?

[Florian-Katerndahl]

Before you merge tihs PR, @davidfrantz I could move the check to the place where LPATH is already checked. This way, everything stays closer together.

If I'm not completely mistaken, the part @ernstste mentioned is this one here: https://github.com/davidfrantz/force/blob/6993764372869d24a7c334a872e681f0e032554e/docs/source/setup/docker.rst?plain=1#L106-L119

[ernstste]

I agree, good point. Getopt doesn't seem to check for arguments starting with a dash, I was under the impression it did.

In most cases, the defined arguments are checked and if they fail the routine, they are printed. Here's an example:

$ force-level1-csd -s --debug meta/ pool/ queue.txt aoi.shp  

Error: Invalid sensor(s) specified
       Sensors provided: --DEBUG
       Valid sensors: S2A,S2B,LT04,LT05,LE07,LC08

The difference here is that the argument isn't printed, so the user is only informed that LPATH is not a valid directory.

The simple fix that keeps consistency with all the other checks would be adding the LPATH var to the print in 388: show_help "$(printf "%s\n " "Log folder does not seem to exist." "Folder provided: $LPATH")"

Adding the new help and purpose functions wouldn't hurt. However, the suggested change adds a wall of text ot every error message, that's not how it was intended. Also the purpose function should follow the standard naming that the other FORCE programs use and be called using -i|--information.

I'll try to implement the suggestions in a clean way in the coming days. Thanks for the suggestions Florian!

[Florian-Katerndahl]

Hey Stefan, the solution you for the LPATH-check is indeed prettier.

I thought that I saw the --purpose flag in some FORCE program, but couldn't find a reference anymore.

If you don't want the detailed description printed everytime, I propose an additional function called show_detailed_help. I think there should be a way of getting all (or most) required information without consulting the docs online.

While I added these changes, if you want to implement them yourself I'll close this PR.

[ernstste]

Hi Florian,

As far as I know the standard flag is -i|--information. @davidfrantz can you confirm? This should be the same across force programs, but maybe it isn't at the moment.

I agree with the point about having a more complete available, it's good practice to be able to explicitely call it and not only have a short version printed when things go wrong. What you describe is basically what I had planned. Keep the current way of printing a very short and concise usage message in case of errors and add a longer help text that also includes the long options and short explanations.

Implementing the changes myself would probably be easiest. However, I appreciate your input and if you value creating changes yourself (to leave a 'footprint' or just because you enjoy doing so), you're of course welcome to create another PR and I will review it/suggest changes if necessary.

[davidfrantz]

Hi @ernstste,

yes, the idea was to have h/v/i for all the programs. This is not yet implemented everywhere, though.

The -i flag was primarily intended to have a dynamic listing of programs and purposes when you simply execute force. This is not yet implemented and the printed messages therein are still hardcoded.

Cheers, David

[Florian-Katerndahl]

All right, I'll either fix the commit history on this PR in the coming days or create a new one!

Cheers, Florian