-
Notifications
You must be signed in to change notification settings - Fork 618
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
stevenyvr987
wants to merge
6
commits into
jonas:master
Choose a base branch
from
stevenyvr987:add_comments_for_bind_commands
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Add comments for bind commands [#809] #926
stevenyvr987
wants to merge
6
commits into
jonas:master
from
stevenyvr987:add_comments_for_bind_commands
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Looks like I missed the case where a user executes a bind option on the prompt line. |
stevenyvr987
force-pushed
the
add_comments_for_bind_commands
branch
from
May 19, 2019 19:10
2f35bf6
to
e6af0ed
Compare
Fixed; force-pushed PR branch; updated my original comment to the PR. |
stevenyvr987
force-pushed
the
add_comments_for_bind_commands
branch
from
May 20, 2019 20:58
e6af0ed
to
5f3b9bf
Compare
Another update of the PR. I've added a new parameter to |
stevenyvr987
force-pushed
the
add_comments_for_bind_commands
branch
from
May 31, 2019 17:57
5f3b9bf
to
9188e6d
Compare
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
force-pushed
the
add_comments_for_bind_commands
branch
from
July 27, 2019 18:49
9188e6d
to
dcfe1fd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 incomments
displayed next toaction
, 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.
If a
bind
is defined in a repo configuration file,tig
will first strip comments by usinggit config --list
before parsing thebind
option. Consequently, comments can't be shown in the help screen forbind
options appearing in repo config files.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.
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 inargv_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.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.
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.
The expected output of two existing test cases have changed and have been corrected. A modest test case is added for the new behaviour.
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.
Simplify the code a bit and trade a bit of space for time by caching key strings during
help_open()
so thathelp_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.