-
Notifications
You must be signed in to change notification settings - Fork 140
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
Refactor: Replace deferred code in yank commands with functions #364
Closed
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
Forgit allows specifying git options in environment variables that are passed along to the individual git commands. We currently treat those as strings. This commit adds a _forgit_parse_array function and uses it to parse all such environment variables into arrays instead. This will allow us to get rid of deferred code, since we can pass the parsed arrays directly to the git commands and don't have to rely on eval.
Removes the deferred code that is used for creating the fzf preview functions and replaces it with _forgit_*_preview functions instead. These functions are exposed as forgit commands so they can be invoked from the fzf subshell. We split the exposed commands into public_commands and private_commands. The only difference between them is that public_commands are mentioned in the help text. This commit changes the flags variable in _forgit_blame from a string to an array. This is necessary to allow the flags to be passed to _forgit_blame_preview as individual arguments.
We often used deferred code to encapsulate git commands and make them reusable. This change removes deferred code for git commands and replaces it with functions instead. Some of the deferred code was used with xargs, which executes it on a subshell. To avoid having to expose the new git functions the same way we do with the preview functions, the usage of xargs in these cases is replaced with either a loop or a single command when possible.
We used to have a variable that was either undefined or contained a piece of deferred code that piped input through emojify when present on the system. To remove the deferred code here, this commit replaces the _forgit_emojify variable with a function that either pipes the input through emojify or through cat, depending on whether emojify is present.
We were using deferred code in git commands in some places without any reason. Each of these deferred code snippets was only executed a single time, so we can replace them with regular git commands. This commit changes how we handle the FORGIT_LOG_GRAPH_ENABLE environment variable. We previously used a variable that stored the --graph flag as a string and unset it, when FORGIT_LOG_GRAPH_ENABLE was set to anything other than true. We now create an empty array and add the --graph flag to it when FORGIT_LOG_GRAPH_ENABLE is unset or true. Doing it this way allows us to build a command line without having to use eval. The outcome is the same as before.
In _forgit_log and _forgit_enter it is possible to diff a single commit/file by pressing enter. We used to store the code that executes the diffs in variables and passed it to fzf as deferred code. This refactor reduces the amount of deferred code by using functions instead of variables.
_forgit_diff and _forgit_add allow editing the currently previewed file in the EDITOR. This used to be handled entirely using deferred code. This commit replaces the deferred code and binds the commands to functions instead.
Many commands allow copying the commit hash or stash name of the current selection to the clipboard. We previously used deferred code to do so. This commit replaces the deferred code and binds these commands to functions instead.
carlfriedrich
approved these changes
Mar 12, 2024
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.
@sandr01d Great, thanks a lot!
cjappl
approved these changes
Mar 13, 2024
Changes have been merged with #326 |
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.
Check list
Description
Many commands allow copying the commit hash or stash name of the current selection to the clipboard. We previously used deferred code to do so. This PR replaces the deferred code and binds these commands to functions instead.
Note
This PR belongs to #326 and resulted from discussions in #324.
Type of change
Test environment