wfxr / forgit

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

[RFC] Split core functions and additional functions #158

Closed wfxr closed 2 years ago

wfxr commented 3 years ago

Check list

Description

As I said in #155, if we keep adding commands into forgit it will become more and more large and unwieldy. This is inconsistent with it's goal. forgit is no comparable with lazygit or gitui etc in terms of completeness of functions, and never it will be. But I fully understand that not commonly used does not mean useless, just like @wren said:

Admittedly, reflog isn't a command that I need often, but when I do there's really no replacement, and I think forgit would be very useful.

So it's time to find a way that can loosely integrate some useful but not commonly used functions into forgit, without forcing people who do not need these functions to pay extra.

A straightforward solution I thought of is to divided function of forgit into two groups. The core part is composed of base utils and the most commonly used git commands, and the additional part can contain some useful but not frequently used commands. One can activate these extra features by setting option like FORGIT_ENABLE_EXTRAS.

Related issues: #155 #157

cjappl commented 3 years ago

I'd like to ping @wren and @D4KU to get their thoughts on this too if they're free.

Here's my 2 cents:

I'm not sure I love the dividing into two pieces. I think we should continue doing the "most used" path, and ignore the rest. This is because the more code we add supporting smaller workflows, we just have more code to have bugs in, without getting "much" in return.

I would think that if you somehow had data on every person's git usage, the commands would break down something like this:

80% of all commands sent to git add commit push log checkout pull diff

19% reset stash clone merge rebase

<1% Everything else

These numbers aren't meant to be exact, but what I'm getting at is that there are some commands that do the massive share of all git commands, and some that are only used in fringe circumstances.

I would vote for something like the following:

D4KU commented 3 years ago

I'm no big fan of a division, either. I'd much rather see a forgit focused on implementing the most-often used commands well than one trying to integrate every git command there is. It's probably hard to draw that line in accordance with everybody, but one would also have to draw a line when splitting up commands. I'm fine with the commands @cjappl listed, including a few like cherry-pick that we already have, and reflog.

wfxr commented 3 years ago

@cjappl @D4KU Thanks for your comments. I aggre with your opinion of not dividing. For compatibility reasons I also prefer keeping all existing functions.

If we want to add a new command, we open up an issue and try to get some number (10?) of thumbs up from the community before implementing it, to make sure we aren't implementing something people won't use.

Another similiar way is the pr that adds new functions in the future will no longer be directly merged, instead one can maintain it in a wiki. If a feature is popular, for example as @cjappl said, get enough thumbs up from the community, we will merge it into built-in functions.

I think these two ways are not in conflict. Maybe they can be used at the same time. Of course the pr already received enough support in the corresponding issue can be merged directly.

cjappl commented 3 years ago

@wfxr Yeah, I think either way is totally fine, and we could do both! Either a feature needs to be approved by X number of thumbs up on the PR, or on the feature request.

I think as an action item for this task (#158) we should update our PR and issue templates to describe this approach, so people know. We should probably recommend people open up issues to discuss their new features before jumping into PRs.

wren commented 3 years ago

I'm personally not a fan of hiding functionality behind feature flags, but I do think that the main file in this plugin can be split up into multiple files containing various functions.

This could help with a few things:

  1. Functions could optionally be made reusable. For example, as I mentioned in another issue, git log and git reflog have nearly identical output. So, it would be possible to have forgit::log() and forgit::reflog() both call a shared function that handles the output. This would help performance since there would be less work performed when a new command is added (provided the new command shares some functionality with current commands, which I think most do).
  2. Autoloading could be implemented (supported by both zsh and bash, and requires separate files). This would allow forgit to add as many functions as needed without worrying about any performance hit for people that don't use them.
  3. This would make it easier to maintain old function names for backwards compatibility, without being permanently tied to them. For example, if the log function is reworked, forgit::log() could be made into a function that simply calls the new function while passing on all arguments (e.g. forgit::log() { new_log_function "$@"; }

Those are just some quick thoughts. I think implementing autoload would probably be the biggest benefit. Please let me know if you'd like me to elaborate on any part of this, or what you think.

D4KU commented 3 years ago

@wren I totally agree. One way to split up the code base that comes to my mind would have a separate file for:

  1. functions executed for the preview window
  2. functions decorating the file list like ga showing the added, deleted markers, etc.
  3. logic executed after the user chooses one or multiple files

Every command would then be just a composition of these three building blocks. Our git stash implementation for example could share function (1) and (2) with ga. It's just (3) that, well, stashes instead of adds.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.