wfxr / forgit

:zzz: A utility tool powered by fzf for using git interactively.
MIT License
4.32k stars 136 forks source link

Refactor: Replace deferred code in enter commands with functions #358

Closed sandr01d closed 3 months ago

sandr01d commented 3 months ago

Check list

Description

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.

Note

This PR belongs to #326 and resulted from discussions in #324. In comparison to #326 I've replaced the $enter_cmd variable in _forgit_diff with a function aswell. The part of it that cds into the repo directory has been removed because this is handled by _forgit_diff_view already.

Type of change

Test environment

sandr01d commented 3 months ago

I've noticed there is still some deferred code left in the pager variables (e.g. $_forgit_enter_pager). I think we should replace these with functions as well. Not in this PR though. We might want to do it after all changes in #326 have been reviewed or even after #326 has been merged.

carlfriedrich commented 3 months ago

@sandr01d Great, thanks for the patch. I think next could be the execute_silent commands, right? What else have we left?

I've noticed there is still some deferred code left in the pager variables (e.g. $_forgit_enter_pager). I think we should replace these with functions as well. Not in this PR though. We might want to do it after all changes in #326 have been reviewed or even after #326 has been merged.

That's a good idea, and I indeed think we can do it after we finished the ongoing review.

sandr01d commented 3 months ago

@carlfriedrich

I think next could be the execute_silent commands, right?

I think we should do _forgit_extract_sha first because that's used in the execute-silent commands a few times.

What else have we left?

Apart from those two, I think only _forgit_quote_files is left. I might split the execute_silent commands into multiple PRs though (edit, yank commit and yank stash).

carlfriedrich commented 3 months ago

@carlfriedrich

I think next could be the execute_silent commands, right?

I think we should do _forgit_extract_sha first because that's used in the execute-silent commands a few times.

What else have we left?

Apart from those two, I think only _forgit_quote_files is left. I might split the execute_silent commands into multiple PRs though (edit, yank commit and yank stash).

Great, let's do it like that!

sandr01d commented 3 months ago

Changes have been merged with #326