vimiv-qt icon indicating copy to clipboard operation
vimiv-qt copied to clipboard

Describe Command Line Arguments

Open jcjgraf opened this issue 4 years ago • 10 comments

Adds a section containing all available command line arguments to the documentation

Todos:

  • [x] Fix -- displayed as -
  • [x] Add examples
  • [x] Improve phrasing
  • [ ] (Re)Think about placement in documentation

Implements the second task from #382

jcjgraf avatar May 11 '21 21:05 jcjgraf

Thanks a lot for starting to work on this! Just let me know when you are ready for a quick review so we can start the discussion :blush:

karlch avatar May 13 '21 09:05 karlch

Content-vies it contains everything. So feel free to have a close look at it. The last two todos are anyways rather for you; if you do not like the phrasing or spot any typos let me know. Also, I have created a dedicated section for the commandline arguments. But maybe you prefers to have it integrated in a pre-existing section like the Getting Started section.

jcjgraf avatar May 13 '21 10:05 jcjgraf

Concedrning the functionality and the content, this PR is done. You have mentioned, that we may want to split the code into multiple files. Do you have anything specific splitting in mind?

I can think of:

  1. Move only ParserArgument
  2. Move in addition _get_arguments, _generate_synopsis, _generate_command
  3. Move in addition generate_commandline_options

What you think?

jcjgraf avatar Aug 08 '21 11:08 jcjgraf

I am not sure concerning the splitting anymore, and really still have to review this properly, sorry for the delays. One thing I noticed though is that the man page looks different now, the SYNOPSIS part now includes some of the * characters which should only be used for formatting. I haven't figured out why this is the case though..

karlch avatar Aug 08 '21 11:08 karlch

Are you talking about docs/manpage/synopsis.srtsrc? Is there way to have a look at the final man page, as vimiv --help seems to be still showing some different version?

Just looking at the synopsis.srtsrc, everything looks fine to me:

Initial:

**vimiv** [**PATH**] [**-h**] [**--help**] [**-f**] [**--fullscreen**] [**-v**] [**--version**] [**-g** *WIDTHxHEIGHT*] [**--geometry** *WIDTHxHEIGHT*] [**--temp-basedir**] [**--config** *FILE*] [**--keyfile** *FILE*] [**-s** *OPTION* *VALUE*] [**--set** *OPTION* *VALUE*] [**--log-level** *LEVEL*] [**--command** *COMMAND*] [**-b** *DIRECTORY*] [**--basedir** *DIRECTORY*] [**-o**] [**--output**] [**-i**] [**--input**] [**--debug** *MODULE*] [**--qt-args** *ARGS*]

This PR:

**vimiv** [**PATH**] [**--help**|**-h**] [**--fullscreen**|**-f**] [**--version**|**-v**] [**--geometry** *WIDTHxHEIGHT*|**-g** *WIDTHxHEIGHT*] [**--temp-basedir**] [**--config** *FILE*] [**--keyfile** *FILE*] [**--set** *OPTION* *VALUE*|**-s** *OPTION* *VALUE*] [**--log-level** *LEVEL*] [**--command** *COMMAND*] [**--basedir** *DIRECTORY*|**-b** *DIRECTORY*] [**--output**|**-o**] [**--input**|**-i**] [**--debug** *MODULE*] [**--qt-args** *ARGS*]

jcjgraf avatar Aug 08 '21 14:08 jcjgraf

You can build the man page via

tox -e man

and then open it directly

man misc/vimiv.1

With this PR I get a final look like this: scrot-2021-08-08_18:55:41 which is somewhat off, but I am not sure why yet.

karlch avatar Aug 08 '21 16:08 karlch

Soon is new year and I sill have quite a few dangling PRs. Therefore, I thought it is a good time to start finish them :blush:

I have gone through all changes of this PR again, rebased it to the current master and fixed the formatting error you have noticed.

There are a few remaining points to discuss:

  • You have mentioned that we may want to split src2rst.py up. Personally, I do not think that is actually that bad. If it is getting out-of-hands when continuing with #382 we can still split it later on. But it is up to you to decide.
  • It would be nice to indicate all possible values for --qt-args. The problem is that I do not know the allowed args (nor did I find them in the PyQt docs). It would be nice to have them generated automaicalls, or if this is not possible, add a link to some external documentation or something. But we can still do that later on I case you are not aware of an simple solution.

Besides the mentioned points, I am done with this PR. Let me know if there is anything to improve or fix.

jcjgraf avatar Dec 26 '21 21:12 jcjgraf

Thanks for your additional effort!

Concerning your two points:

  • I agree, we can keep it for now, but probably need re-formatting sooner or later.
  • I am also not sure where one would find all possible values, but probably many of them would not be useful for us anyway. One example would be the -reverse flag which causes vimiv to look horribly broken :laughing: So I would ignore this for now and stick to documenting useful (working) options.

I am a bit confused with the state of this PR though, when I try to build anything I get an AttributeError:

man run-test: commands[0] | /home/christian/Coding/VimivQt/scripts/src2rst.py
Traceback (most recent call last):
  File "/home/christian/Coding/VimivQt/scripts/src2rst.py", line 182, in <module>
    def _get_arguments(argparser: parser.argparser.ArgumentParser) -> List[ParserArgument]:
AttributeError: module 'vimiv.parser' has no attribute 'argparser'. Did you mean: 'argparse'?

and when I fix this simple error I get more havoc

man run-test: commands[0] | /home/christian/Coding/VimivQt/scripts/src2rst.py
generating statusbar modules...
generating commands...
generating settings...
generating keybindings...
Traceback (most recent call last):
  File "/home/christian/Coding/VimivQt/scripts/src2rst.py", line 333, in <module>
    generate_commandline_options()
  File "/home/christian/Coding/VimivQt/scripts/src2rst.py", line 105, in generate_commandline_options
    f.write(_generate_synopsis_man(arguments))
  File "/home/christian/Coding/VimivQt/scripts/src2rst.py", line 228, in _generate_synopsis_man
    command = "\ \|\ ".join(arg.get_name_metavar())
TypeError: ParserArgument.get_name_metavar() missing 2 required positional arguments: 'name_formatter' and 'metavar_formatter'

Am I missing something?

karlch avatar Jan 06 '22 13:01 karlch

I am sorry, I must have lost a commit while rebasing/cleanup. Now everything (should) works!

jcjgraf avatar Jan 07 '22 19:01 jcjgraf

quickly rebased to solve merge conflicts

jcjgraf avatar Jan 08 '22 10:01 jcjgraf

Thanks again for working on this! When looking through, I thought there were some small parts of code that could be simplified (were you on Haskell drugs at the time? :wink: :yum: ). Thus, I quickly did the changes, and also added the settings part in #657.

Hope this is okay with you! :blush:

If you like, you can take a quick look at #657 and give your okay :blush:

karlch avatar Jul 07 '23 13:07 karlch