use command to “protect” further commands
#3462
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
This commit causes all simple commands that are not built-ins or functions to be invoked via `command` in order to protect them from alias substitution or from accidentally taking functions of the same name. It was decided to not “protect” `fzf` and `fzf-tmux` for now. Maybe a better solution should be implemented for that in the future. Signed-off-by: Christoph Anton Mitterer <[email protected]>
|
@junegunn … I did some (very very limited) testing now... and the test suite also runs through. So once @step- has made his fix of the issue described at #3313 (comment), I'd rebase this branch and we could IMO merge it (after you double and triple checked it ;-) ). |
step-
added a commit
to step-/fzf
that referenced
this pull request
Oct 6, 2023
* Use the all-compatible mawk `-W version` option. junegunn#3313 (comment). * Do not remap the history key if no dependent commands is installed (perl, awk or mawk in this order). * Run the command and not a function consistently with junegunn#3462. The version check bash code relies on the following mawk source code, extracted from mawk 1.3.4 20230322. ``` version.c: 18- #include "init.h" 19- #include "patchlev.h" 20- 21: #define VERSION_STRING \ 22- "mawk %d.%d%s %s\n\ 23- Copyright 2008-2022,2023, Thomas E. Dickey\n\ 24- Copyright 1991-1996,2014, Michael D. Brennan\n\n" .... 30- void 31- print_version(FILE *fp) 32- { 33: fprintf(fp, VERSION_STRING, PATCH_BASE, PATCH_LEVEL, PATCH_STRING, DATE_STRING); 34- fflush(fp); 35- 36- #define SHOW_RANDOM "random-funcs:" patchlev.h: 13- /* 14- * $MawkId: patchlev.h,v 1.128 2023/03/23 00:23:57 tom Exp $ 15- */ 16: #define PATCH_BASE 1 17- #define PATCH_LEVEL 3 18- #define PATCH_STRING ".4" 19- #define DATE_STRING "20230322" ```
junegunn
reviewed
Oct 7, 2023
junegunn
added a commit
that referenced
this pull request
Oct 7, 2023
* Use the all-compatible mawk `-W version` option. #3313 (comment). * Run the command and not a function consistently with #3462. The version check bash code relies on the following mawk source code, extracted from mawk 1.3.4 20230322. ``` version.c: 18- #include "init.h" 19- #include "patchlev.h" 20- 21: #define VERSION_STRING \ 22- "mawk %d.%d%s %s\n\ 23- Copyright 2008-2022,2023, Thomas E. Dickey\n\ 24- Copyright 1991-1996,2014, Michael D. Brennan\n\n" .... 30- void 31- print_version(FILE *fp) 32- { 33: fprintf(fp, VERSION_STRING, PATCH_BASE, PATCH_LEVEL, PATCH_STRING, DATE_STRING); 34- fflush(fp); 35- 36- #define SHOW_RANDOM "random-funcs:" patchlev.h: 13- /* 14- * $MawkId: patchlev.h,v 1.128 2023/03/23 00:23:57 tom Exp $ 15- */ 16: #define PATCH_BASE 1 17- #define PATCH_LEVEL 3 18- #define PATCH_STRING ".4" 19- #define DATE_STRING "20230322" ``` Co-authored-by: Junegunn Choi <[email protected]>
junegunn
reviewed
Oct 7, 2023
calestyo
force-pushed
the
protect-commands-in-shell-scripts
branch
from
98b6dfd to
08948ac
Compare
|
Rebased the commit on current master, though it still has the |
calestyo
force-pushed
the
protect-commands-in-shell-scripts
branch
from
08948ac to
2c35c19
Compare
junegunn
reviewed
Oct 11, 2023
|
Merged, thanks! |
kugel-
added a commit
to kugel-/fzf
that referenced
this pull request
Oct 1, 2024
With the current code, I get this command cat: command not found when using any keybinding that does _fzf_complete. Seen on Ubuntu 24.04.1 (GNU bash, version 5.2.21(1)-release (x86_64-pc-linux-gnu)) Fixes: 561e0b0 ("[bash] Use `command` to “protect” further commands (junegunn#3462)") Signed-off-by: Thomas Martitz <[email protected]>
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.
Hey.
This would be for #3458.
The current commit (hopefully) adds
commandto all commands that are not built-ins or functions.I haven't really tested it yet ;-) ... and guess anyway that it will be rather quite some effort to test all cases where
commandis now used.A future commit might use
builtinto protect further things likeprintforecho. But arguably, it's always a trade off how far we go here (code readability vs. actually adding some benefit).But I do think, that at least "regular" (non-built-in) commands should all be "protected".
Also, I made no effort to do the same for fish or zsh. Since this adds no real new feature, I hope this is okay.
But I did notice that all zsh files have:
fzf/shell/completion.zsh
Line 70 in 488a236
fzf/shell/key-bindings.zsh
Line 35 in 488a236
Not really sure how that works, but shouldn't it anyway make the
commandsuperfluous, at least in terms of aliases (probably not in terms of functions)?