bk138 / gromit-mpx

Gromit-MPX is an on-screen annotation tool that works with any Unix desktop environment under X11 as well as Wayland.
GNU General Public License v2.0
975 stars 81 forks source link

Add option to change tools on the command line #189

Open pascal-niklaus opened 7 months ago

pascal-niklaus commented 7 months ago

This refers to https://github.com/bk138/gromit-mpx/issues/187

Changes:

(1) Command-line option added that allows changes of the tools while gromit is running, following the syntax of the config file. Example:

  gromit-mpx --change-tool '"default"=PEN(color="green" size=4 arrowsize=2)'

(2) Updated man page to include this option, and also added documentation for --line that was missing

(3) Implementation details: a new file parser.c/.h was created that contains functions to scan the pen definitions. These functions are now called from config.c and from the callback invoked when changing these setting on-the-fly when gromit is running.

pascal-niklaus commented 7 months ago

I have some comments at the respective code lines and some general thoughts.

The general ones:

* So this is essentially the meta-tool called toolswitcher devised in [Make the various tools selectable by the system tray icon and from control #91](https://github.com/bk138/gromit-mpx/issues/91) plus that it can redefine tools and minus that it's CLI only?

Yes, this sounds a bit like it, except for the absence of a tray icon part.

* My general goal is to make the app easy for non-pro users to use, so [Add UI for creating/editing tools and mapping to buttons and/or maybe keys #110](https://github.com/bk138/gromit-mpx/issues/110) is the go-to solution for a user interface; also the app should be usable w/o any third-party tools like key-mappers and the like. So, _in general_, I would not add too much special CLI features _unless_ they're small and easy to maintain. So I guess I would merge this if you can slim it down (removing the refacto into parse.c e.g.), otherwise rather not. In any case, please don't be put off, I very much appreciate any contribution!

I can easily move the content of parser.c into config.c, and call these functions from main.c for the command-line option. That part is straightforward.

Additional thoughts:

One of my use cases is with a graphics tablet and presentations in full-screen mode, with no tray icon visible. With extra tools (colors, rect, line, single and double arrows, etc) at some stage the number of modifier keys is quickly exhausted.

Key mappings would work, but then it would be great to be able to distinguish keystrokes from an external keyboard (like a numeric keypad for tool changes) and the "normal" keyboard. That would be along the line with the option to have two mouse cursors, one for gromit (driven by e.g. a tablet pen) and another one for regular work.

bk138 commented 7 months ago

Regarding this one, I think a CLI toolswitcher is something we would like to have.

Not sure if a tool-redefiner would add too much extra complexity. I'd rather have one single place (the .cfg or later on, the .ini file after #110) where definitions are stored and read from for switching/drawing purposes.

pascal-niklaus commented 7 months ago

Regarding this one, I think a CLI toolswitcher is something we would like to have.

Not sure if a tool-redefiner would add too much extra complexity. I'd rather have one single place (the .cfg or later on, the .ini file after #110) where definitions are stored and read from for switching/drawing purposes.

Another way to think about this is that the diversity of tools that gromit offers will keep growing (at least I have some more ideas in that direction ;-) ). Given this, I think it would be good to allow changing the individual attributes of the tools separately (e.g. tool type, color, line width etc).

For example, I may wish to draw in yellow, and then change from a PEN to a RECT or LINE (all still yellow). Later, maybe I want to change the color to green, or just modify the line width (again, preserving these choices for the future tools).

With individual tool definitions, an enormous variety of tools would need to be defined to accommodate all combinations (and one would have to remember which is which).

I understand that this is a different approach than providing a set of a few carefully chosen predefined tools, like this is now implemented with the modifier keys.

For the CLI, my suggestion is therefore to provide an option to change the individual attributes of the default tool.

This is based on my experience with a screenshot annotation tool I frequently use, ksnip. ksnip allows changing tools with a single key press (e.g. L = line, A= arrow, D = double arrow, R = rect etc), which in principle allows for a very smooth workflow. However, in each and every tool the color and line width has to be set separately, which IMO spoils the user experience.

But maybe the two approaches could be combined? For example, some tools could be defined in the .ini file and selected by their number, and the default tool attributes changed as outlined above?

pascal-niklaus commented 6 months ago

I have now integrated the content of "parser.c/parser.h" into config.c/config.h, as suggested.

Now, two command-line options are available for tool changes:

  1. Using --change-tool or -T, a complete tool definition can be changed, for example like so:

    gromit-mpx --change-tool '"default"=PEN(color="Limegreen" size=4 arrowsize=2 arrowtype="end")'
  2. Using --change-attribute or -A an individual attribute of a tool can be changed, keeping all others. This way, it for example is possible to change from PEN to LINE and keep color, width, arrows etc, or to just change the color and line width, regardless of the tool. Examples are

    gromit-mpx --change-attribute '"default"=(color="Limegreen" size=4)'   # change just attributes keeping tool
    gromit-mpx --change-attribute '"default"=PEN(color="cyan")'            # change tool and some attributes
    gromit-mpx --change-attribute '"default"=RECT'                         # change just the tool
pascal-niklaus commented 6 months ago

Now all changes requested should be addressed, and the PR be compatible with the current master.

Re your question: