sidestep readline when executing a script #703
Merged
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
jonas
reviewed
Jul 30, 2017
src/prompt.c
Outdated
| if (cmd && *cmd && !argv_from_string(argv, &argc, cmd)) { | ||
| report("Too many arguments"); | ||
| return REQ_NONE; | ||
| } | ||
|
|
||
| if (!cmd || !*cmd) { | ||
| report_clear(); | ||
| if (!is_script_executing()) | ||
| report_clear(); |
There was a problem hiding this comment.
Is this conditional behavior needed?
There was a problem hiding this comment.
No, this part is not important.
|
How much slower was it? Persistence in the user Tig history file is definitely a big. |
|
The speed penalty depends on configuration and can be extreme: $ touch ~/.inputrc
$ printf 'set blink-matching-paren on\n' >> ~/.inputrc
$ TIG_SCRIPT=<(printf ':echo %%(commit)\n:echo %%(commit)\n:echo %%(commit)\n:echo %%(commit)\n:echo %%(commit)\n:quit\n') /usr/bin/time ./src/tig
2.70 real 0.09 user 0.05 sys
$ git checkout script-sidestep-readline && make > /dev/null
$ TIG_SCRIPT=<(printf ':echo %%(commit)\n:echo %%(commit)\n:echo %%(commit)\n:echo %%(commit)\n:echo %%(commit)\n:quit\n') /usr/bin/time ./src/tig
0.17 real 0.08 user 0.04 sys |
rolandwalker
force-pushed
the
script-sidestep-readline
branch
6 times, most recently
from
65facbe
to
7a0be90
Compare
jonas
reviewed
Aug 11, 2017
| @@ -127,6 +127,20 @@ prompt_default_handler(struct input *input, struct key *key) | |||
| } | |||
|
|
|||
| static enum input_status | |||
| prompt_script_handler(struct input *input, struct key *key) | |||
There was a problem hiding this comment.
I am not sure if defining this is actually necessary. read_prompt_incremental allows to pass a NULL handler.
There was a problem hiding this comment.
There might be a smarter way to do it, but the NULL handler doesn't recognize newlines.
rolandwalker
force-pushed
the
script-sidestep-readline
branch
2 times, most recently
from
2718278
to
14528e5
Compare
rolandwalker
force-pushed
the
script-sidestep-readline
branch
from
14528e5
to
6e2a8a2
Compare
|
Thanks. |
rolandwalker
added a commit
to rolandwalker/tig
that referenced
this pull request
May 21, 2018
Readline was causing major perf issues when interpreting a script, and inappropriately storing each script line in ~/.tig_history.
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.
Readline was causing major perf issues when interpreting a script, and inappropriately storing each script line in
~/.tig_history.