vimiv-qt
vimiv-qt copied to clipboard
Describe Command Line Arguments
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
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:
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.
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:
- Move only
ParserArgument - Move in addition
_get_arguments,_generate_synopsis,_generate_command - Move in addition
generate_commandline_options
What you think?
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..
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*]
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:
which is somewhat off, but I am not sure why yet.
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.pyup. 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.
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
-reverseflag 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?
I am sorry, I must have lost a commit while rebasing/cleanup. Now everything (should) works!
quickly rebased to solve merge conflicts
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: