-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[fish] Small clean-ups and optimizations #4259
Merged
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
Append all arguments after the first one, so that functions don't have to pass all appending options as a single string. Also, output everything as a single string (an array of one item).
The FZF_TMUX variable check has already been changed from numeric to string, so there is no need to set it to 0 if it's empty or undefined.
The __fzf_get_dir function was called only once, and was basically a single command in a while loop.
- Remove check/set of FZF_TMUX_HEIGHT variable. It is already done by __fzf_defaults. - Remove unnecessary begin/end block. - Simplify result variable check. - Insert file names using a single call to commandline.
- Remove check/set of FZF_TMUX_HEIGHT variable. It is already done by __fzf_defaults. - Remove unnecessary begin/end block. - Pass all fzf options (except query) through FZF_DEFAULT_OPTS variable.
- Remove check/set of FZF_TMUX_HEIGHT variable. It is already done by __fzf_defaults. - Remove unnecessary begin/end block. - Simplify result variable check. - Set the command line using a single call to commandline.
Use single check for each default command variable.
Move the helper functions to the top of the main function, and the main function commands (bind command) to the bottom.
17e27fe
to
aaacae4
Compare
junegunn
approved these changes
Feb 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, they all look good to me. Is there anything else you want to address?
No, I think that's all. Thanks! |
Rebased and merged. Thanks! |
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
Minor all-around code improvements and clean-ups. No actual changes to the operation of the script. Tested to work with fish versions 3.1.2, 3.7.1 and 4.0b1-430-g5e38a2a46.
Independently of these changes, even though everything seems to work with the beta version 4.0 of fish, there is (again) one test that inconsistently fails (only with v4):