Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add comments for bind commands [#809] #926

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

stevenyvr987
Copy link
Contributor

@stevenyvr987 stevenyvr987 commented May 18, 2019

A user may have defined numerous and complex key bindings. The user will show the help screen to be reminded of the key bindings. The user will see keybindings for built-in actions accompanied by help text, because they are built into the source code. However, the user will see user-defined key bindings without help text, because any comments typed in the configuration file are stripped when parsed. Without seeing the comments, the user may not remember the meaning of a key binding from seeing just the action name.

The PR adds code to enable comments in bind options to be shown in the help screen. For example, bind keymap key action # comments will result in comments displayed next to action, in the same way that help text for built-in actions are shown. Also, if a / search is done, the PR adds code to ensure that comment text are searched.

There are two exceptions.

  1. If a bind is defined in a repo configuration file, tig will first strip comments by using git config --list before parsing the bind option. Consequently, comments can't be shown in the help screen for bind options appearing in repo config files.

  2. If a bind is defined on the prompt line, tig will not parsed any comments introduced by a hash mark, because it's not expected the user will type any. Consequently, comments will be parsed as command parameters.

The PR is organized into several commits. See details in the various commit messages.

  1. A prepatory commit. Add a new function to argv.c similar to argv_to_string_alloc(), but it adds a prefix to the allocated text buffer. In the process, a bug is discovered and fixed in argv_to_string_alloc(): if a non-empty separator is specified, argc will fail to count the addition of separators. Since existing calls of the function always specify empty separators, there has been no impact from the bug.

  2. Add code to show the comment text in the help screen. At this stage, comment text is shown as free-form text and not aligned with built-in help text.

  3. Add code to align comment text when shown in the help screen; the code uses the function introduced in the first commit. Only keybindings with comment text will participate in calculating the field width; keybindings with no comments but with long action names may overflow the field.

  4. The expected output of two existing test cases have changed and have been corrected. A modest test case is added for the new behaviour.

  5. Make sure that new comment text for keybindings can participate in grep searches, ie, if the user initiates a / search on the prompt line, matches in comment text should show up. Unfortunately, I can't come up with a unit test for this.

  6. Simplify the code a bit and trade a bit of space for time by caching key strings during help_open() so that help_draw() doesn't need to recalculate them when drawing a help line.

I can follow up with another commit to make the necessary changes to documentation.

@stevenyvr987
Copy link
Contributor Author

Looks like I missed the case where a user executes a bind option on the prompt line.
Will fix and update the PR.

@stevenyvr987 stevenyvr987 force-pushed the add_comments_for_bind_commands branch from 2f35bf6 to e6af0ed Compare May 19, 2019 19:10
@stevenyvr987
Copy link
Contributor Author

Fixed; force-pushed PR branch; updated my original comment to the PR.

@stevenyvr987 stevenyvr987 force-pushed the add_comments_for_bind_commands branch from e6af0ed to 5f3b9bf Compare May 20, 2019 20:58
@stevenyvr987
Copy link
Contributor Author

Another update of the PR. I've added a new parameter to set_option(), which allows me to pass in comment text in a simpler way.

@stevenyvr987 stevenyvr987 force-pushed the add_comments_for_bind_commands branch from 5f3b9bf to 9188e6d Compare May 31, 2019 17:57
1. Fix bug in argv_to_string_alloc(),
   which did not increase buffer size to account for number of separators.

2. Improve coding
   a. Initialize buffer size to one to account for terminator,
      which avoids the need to later increment by one.
   b. Call concat_argv() directly, instead of calling argv_to_string(),
      which is a wrapper.

3. Add similar function argv_to_string_alloc_prefix(),
   which preloads string buffer with a prefix string.
1. keys.h

   Add a run_request.help field, analogous to the req_info.help field.

2. options.h

   Add a parameter to set_option() for passing any comment text.

3. options.c

   a. read_option()

      Identify any #comment text string that may have accompanied an option command,
      and pass it into set_option(). The #comment text string is trimmed of any white space.

   b. set_option()

      set_option() dispatches to specific bind commands.
      Pass any given comment text into the option_bind_command(),
      and ignore comment text for the other bind commands.

   c. option_bind_command()

      Pass comment text into add_run_request().

   d. read_repo_config_option()

      Specify the option command to use with "color", "bind", or "set" names
      instead of specific function names.

   e. set_repo_config_option()

      Refactor to call set_option(), the dispatcher for specific bind commands,
      and call with comment text pointer set to NULL,
      because comment text is never available from a repo configuration file.

4. prompt.c

   a. run_prompt_command()

      Call set_option() with comment text pointer set to NULL,
      because comment text is not parsed on a prompt line.

5. keys.c

   a. add_run_request()

      Duplicate comment text into the new run_request.help field.

6. help.c

   a. help_draw()

      Draw the run_request.help text using draw_text(), analogous to drawing req_info.help.
Add code to align comment text when shown in the help screen. Comment text are drawn on the right-hand side of an 'action' field.

Change the calculation of field width: only key bindings with comment text will participate in calculating the field width. Consequently, key bindings with no comments but with unusually long action names are allowed to overflow the field.

1. keys.h

   Add a name field to struct run_request, analogous to the name field in struct req_info.

2. keys.c

   a. add_run_request()

      * Load run_request.name with a displayable version of **argv, using new function argv_to_string_alloc_prefix().
      * Include run_request.flags as a string prefix.

3. help.c

   a. help_keys_visitor()

      * If run_request.help text is present, update help_state.name_width with strlen(run_request.name), so that drawing run_request.help will be aligned.
      * We can also do the same for req_info.help text: calculate the maximum field width only if req_info.help text is available.

   b. help_draw()

      * Analogous to drawing req_info.name as a LINE_HELP_ACTION, draw run_request.name using draw_field(), which replaces the current method of drawing **argv using draw_formatted(). We do this only if run_request.help text is present, otherwise, we draw run_request.name as free-form text.
      * Do the same for req_info.name. In this way, the code for drawing both types of requests are more similar.
1. help/default-test

   The action name field is wider causing re-alignment of help text.

2. tigrc/parse-test

   Comment for a key binding is now exposed as help text.

3. help/user-command-comment-test

   Add a new test for defining user comments for user commands.
1. help.c

   a. help_grep()

      For keybinding for each run request, include any help text in the text line so that it can participate in grep operations.
…oes not need to recalculate them

1. help.c

   a. struct help

      Add a key field to cache key strings calculated during help_open() so that help_draw() does not need to recalculate.

   b. help_keys_visitor()

      Cache the available key string in help.key.

   c. help_draw(), help_grep()

      Use cached help.key string instead of recalculating the key string.

   d. help_open()

      Free any key strings that have been cached in the help struct to prevent memory leak during a refresh of help screen.
@stevenyvr987 stevenyvr987 force-pushed the add_comments_for_bind_commands branch from 9188e6d to dcfe1fd Compare July 27, 2019 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant