forgit icon indicating copy to clipboard operation
forgit copied to clipboard

Add completions for forgit aliases

Open carlfriedrich opened this issue 3 years ago • 5 comments

This is my shot for porting @cjappl 's changes of #231 to bash. Not every forgit function supports arguments at the moment, but at least for the ones that support it completion is a useful thing, so I added it for all functions at once, just like in fish.

@wfxr What do you think? And do you know how this could be implemented for zsh?

Tested all completions in bash.

Check list

  • [x] I have performed a self-review of my code
  • [x] I have commented my code in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation

Description

Type of change

  • [ ] Bug fix
  • [x] New feature
  • [ ] Refactor
  • [ ] Breaking change
  • [ ] Documentation change

Test environment

  • Shell
    • [x] bash
    • [ ] zsh
    • [ ] fish
  • OS
    • [x] Linux
    • [ ] Mac OS X
    • [ ] Windows
    • [ ] Others:

carlfriedrich avatar Sep 13 '22 11:09 carlfriedrich

Cool! I had no idea that bash supported such a thing 👀

cjappl avatar Sep 13 '22 14:09 cjappl

@wfxr What do you think? And do you know how this could be implemented for zsh?

@carlfriedrich Good idea! I'm not familiar with this but you can refer to fzf: https://github.com/junegunn/fzf/tree/master/shell

There are two suggestions:

  1. Complete the original functions instead of the aliases?
  2. Move the completion code into a separate file like many other tools to avoid code bloat.

wfxr avatar Sep 14 '22 02:09 wfxr

@wfxr Thanks for your feedback.

1. Complete the original functions instead of the aliases?

Are there actually people who use the functions directly? Or does zsh complete aliases automatically equally to the commands they alias to? Bash at least doesn't.

2. Move the completion code into a separate file like many other tools to avoid code bloat.

Is this compatible with the oh-my-zsh plugin system? Where should these completions go within the forgit tree then?

Since we're mainly re-using existing git completions, I don't think the code gets bloated too much. And the fish code includes the completions directly in the code as well. But I'm open for discussion here.

carlfriedrich avatar Sep 14 '22 07:09 carlfriedrich

@carlfriedrich Thank you. Serveral lines don't make the code bloated but many times of serveral lines will do. I prefer to keep the single main file short and efficient. Not just for running efficiently, but also loading efficiently, and reading efficiently.

Are there actually people who use the functions directly? Or does zsh complete aliases automatically equally to the commands they alias to? Bash at least doesn't.

Someone use custom aliases instead the default. zsh does support complete aliases the same as it's original command.

Is this compatible with the oh-my-zsh plugin system?

IDK but should not a problem since there are many pure completion plugins in omz. Also most plug managers support to specify the completion file path when loading the plugin.

Where should these completions go within the forgit tree then?

A new directory in root named completions is ok for me.

wfxr avatar Sep 14 '22 08:09 wfxr

@carlfriedrich Thank you. Serveral lines don't make the code bloated but many times of serveral lines will do. I prefer to keep the single main file short and efficient. Not just for running efficiently, but also loading efficiently, and reading efficiently.

@wfxr OK, I get your point here.

Are there actually people who use the functions directly? Or does zsh complete aliases automatically equally to the commands they alias to? Bash at least doesn't.

Someone use custom aliases instead the default. zsh does support complete aliases the same as it's original command.

Ah, good to know. I will consider this.

Where should these completions go within the forgit tree then?

A new directory in root named completions is ok for me.

Alright, I will transfer the completions accordingly.

carlfriedrich avatar Sep 14 '22 10:09 carlfriedrich

@wfxr So here's the updated PR with my attempt to provide a separate completion file for bash. It completes the following variants of calling forgit:

  1. forgit functions (e.g. forgit::add)
  2. shell aliases (e.g. ga)
  3. git-forgit wrapper calls (e.g. git forgit add)
  4. git-forgit aliases (e.g. git a with alias.a=forgit add in git config)

The only change left in the main forgit file is the export of all the alias definitions, because that's the only way of knowing these in the completion file. Alternative would be to redefine them over there, but then we would have hard-coded them twice, which bears the risk of running out of sync.

What do you say?

Should we include the completion script in our shellcheck test?

And should/can we implement a similar solution for zsh?

carlfriedrich avatar Oct 03 '22 14:10 carlfriedrich

@wfxr I updated this PR to the current master. Do you mind if I merge this? It adds completions for bash in a dedicated file. I have actually been using these for several weeks now, so they are proven working. The patch should not interfere with zsh or fish environments.

carlfriedrich avatar Dec 12 '22 18:12 carlfriedrich

I decied to adapt this PR to support the following two completions only:

  • git forgit calls (e.g. git forgit add)
  • git forgit aliases (e.g. git a with alias.a=forgit add in git config)

These are my only actual use cases and they don't require any changes in the code, so I consider this PR minimal obstrusive and will merge it directly now.

For the plugin functions/aliases completion I will create another PR.

carlfriedrich avatar Jan 11 '23 09:01 carlfriedrich