git action for creating and checking out a new branch

[smithbm2316]

• Added a new action called actions.git_create_branch that will allow the user to create and checkout a new branch if it doesn't already exist
• Added some basic docstrings for the other git actions

I set the default binding to <C-a> to trigger this action when using the git_branches picker, but am happy to change that if people don't like it. The best mnemonic keymapping I could think of was <C-c> as in checkout or create, but that obviously will not work as a binding lol. I'm very much open to suggestion on the binding, my thought process was <C-a> could be thought of as add new branch, even if that doesn't really match the git command itself (since I used git checkout -b [new-branch]).

[kkharji]

Although c-a is beginning of the line for me, but I hardly see any need for it in git picker. LGTM

cc @elianiva

[smithbm2316]

@tami5 Ooh yes good old readline bindings, I forgot about that. I think it would be good to use something else perhaps, that's something I know a lot of people have ingrained in their fingers! Let me think about an alternate binding and check back in later with some options.

[smithbm2316]

Alright I have two ideas for the mapping:

1. Map to <C-u> by default, since at the moment we are just disabling <C-u> altogether in the git_branches picker. I couldn't come up with any bindings honestly that had a good mnemonic for remembering the mapping easily, but if people don't like it they can always override it in their config.
2. Just add the action and remove the default mappings, add some docs for this picker that if someone wants to use it to create branches, they can setup a custom attach_mappings() for it in their config. I would not prefer this as I think that this would lower the ability for people to discover this functionality, but it is a fine option too I think.

Either way, let me know what your thoughts are. I also added a confirmation prompt to make sure a user doesn't accidentally hit the mapping and create a branch by accident, similar to how the git_delete_branch action works.

I think I will work on some general docs next for all the default mappings for the different builtin pickers, as afaik you really can't find out about those mappings unless you dig into the source code (unless I am mistaken and just have not seen where that is easily able to be found).

[smithbm2316]

cc @elianiva @tami5

[kkharji]

Alright I have two ideas for the mapping:

1. Map to <C-u> by default, since at the moment we are just disabling <C-u> altogether in the git_branches picker. I couldn't come up with any bindings honestly that had a good mnemonic for remembering the mapping easily, but if people don't like it they can always override it in their config.

2. Just add the action and remove the default mappings, add some docs for this picker that if someone wants to use it to create branches, they can setup a custom attach_mappings() for it in their config. I would not prefer this as I think that this would lower the ability for people to discover this functionality, but it is a fine option too I think.

I'm kinda leaning towards the second option as it is what telescope about "customizability" however a default as you stated might be a important. I agree customizability sucks and its kinda hard. With #582 I attempt to solve this issue, hopefully by the end of the week I'll start working on it.

Either way, let me know what your thoughts are. I also added a confirmation prompt to make sure a user doesn't accidentally hit the mapping and create a branch by accident, similar to how the git_delete_branch action works.

Hmm since we're adding not deleting perhaps is a bit robust to confirm, no? adding != deleting. So perhaps it would be better without confirmation.

I think you had it right from the first time. c-a makes sense and can be easily remembered. I highly doubt that any one would use c-a to go to the beginning of the line.

I think I will work on some general docs next for all the default mappings for the different builtin pickers, as afaik you really can't find out about those mappings unless you dig into the source code (unless I am mistaken and just have not seen where that is easily able to be found).

☺️ yah one of the current shortcomings. last time I worked on the readme I forgot to to create a table with all the available action then. It would be exterminated helpful if you managed to document those cluttered lists of actions 🤣.

[smithbm2316]

I'm kinda leaning towards the second option as it is what telescope about "customizability" however a default as you stated might be a important. I agree customizability sucks and its kinda hard. With #582 I attempt to solve this issue, hopefully by the end of the week I'll start working on it.

Yeah, I think that's sensible. My thought process for adding a default was that since there is already a default for deleting a branch, it would seem a bit odd to me as a new Telescope user to not have a default mapping for adding a new branch. But I am perfectly happy with leaving the keymapping up to the user, I can easily add a custom function to my own config for it.

Hmm since we're adding not deleting perhaps is a bit robust to confirm, no? adding != deleting. So perhaps it would be better without confirmation.

I think you had it right from the first time. c-a makes sense and can be easily remembered. I highly doubt that any one would use c-a to go to the beginning of the line.

I can certainly revert it back to <c-a>, as I do think that is more intuitive. I just thought you had a valid point that <c-a> is a common keybinding that people use to jump to the beginning of a line when editing text, so I'd rather not make people wonder why their mental mapping for jumping to the beginning of a line no longer works, only when they are using the git_branches picker. I can easily see a world where a new Telescope user would be confused by that 😅, which if possible I'd always want to avoid if I can. But there really just aren't any other available Ctrl-key-based keymappings that really make any kind of sense lol so it may be fine for this instance. Though, now that I added in the confirmation prompt for creating a new branch, at least if somebody accidentally pressed <c-a> mistakenly or to try and jump to the beginning of the line, they wouldn't accidentally create a branch immediately, they'd have to confirm their choice with 'y' to make that happen. So there is a safeguard there now for that potential issue.

☺️ yah one of the current shortcomings. last time I worked on the readme I forgot to to create a table with all the available action then. It would be exterminated helpful if you managed to document those cluttered lists of actions 🤣.

Well I am still pretty new to Lua, and still kinda getting my bearings inside of the Telescope codebase, so this may be a perfect place for me to help! I consider myself to be a pretty good writer, and am probably still new enough to Telescope that I can explain some of those in decent enough detail that a newbie could understand what I write! I'll look into working on that over the coming days when I have the chance. I really was only able to make this PR by just refactoring the git_delete_branch action, I would have had no idea how to write this from scratch 🤣

[elianiva]

LGTM! We can probably merge after you add the docs for those actions that Tami mentioned, if you want.

[smithbm2316]

@elianiva @tami5 I just switched the default mapping back to <c-a> for now. If you're happy with this, I don't have any other changes to make. However, I'm happy to remove the default binding if we don't want to give one for now, and then I can just add some info in some docs about how to create the binding or something of the sort so people know about it before we merge.

Sidenote: if you see the asciicast video I responded to elianiva with on the review, do either of you have any idea how to fix the weird spacing that happens with the error messages I print()? I had no idea how to try and either clear the previous message before printing a new one, or just fix the spacing. It was driving me crazy 🤣!

Edit: damn elianiva you're fast! beat me to my own response lol. Do you want me to add docs I mentioned in this PR or open a new one with that? Either is fine with me. It might take me a while to get through writing some of that up for all the builtins.

[elianiva]

do either of you have any idea how to fix the weird spacing that happens with the error messages I print()?

It only happens when we print the message as soon as Telescope gets closed. Wrapping it with vim.defer_fn(print_stuff_here, 1) or vim.schedule(print_stuff_here) works but they seem hacky to me, I'm not sure.

Do you want me to add docs I mentioned in this PR or open a new one with that?

I think opening a new one is better so we keep this PR small and we can merge it sooner.

[kkharji]

Yes I agree this PR should be merged for now and later on we can collaborate more on documening available action. Thanks @smithbm2316 tag me whenever you need help with something

[smithbm2316]

It only happens when we print the message as soon as Telescope gets closed. Wrapping it with vim.defer_fn(print_stuff_here, 1) or vim.schedule(print_stuff_here) works but they seem hacky to me, I'm not sure.

Gotcha. After looking at the help docs for both of those I can see why you'd feel those are a bit hacky. I think at least wrapping the case where we have a git error with one of those might be a decent idea because of how awful the formatting is, but it's your call if you want me to try that or not. The other cases the formatting is not so bad, just a bit weird. But a git error (see the asciinema vid) really gets formatted in a hard to read way imo.

I think opening a new one is better so we keep this PR small and we can merge it sooner.

I agree!

Edit: and by the time I finished typing this out on my phone, it's merged of course 😂. Thanks both of you for the help. I'll ping you when I have at least a WIP PR ready to go for those docs we've talked about.

[kkharji]

Sidenote: if you see the asciicast video I responded to elianiva with on the review, do either of you have any idea how to fix the weird spacing that happens with the error messages I print()? I had no idea how to try and either clear the previous message before printing a new one, or just fix the spacing. It was driving me crazy 🤣!

Damn I just noticed it, not sure what's the issue.

I think this action can later be improved to close the prompt after clicking c-a -> create new popup in center of the screen with a message and a field to input branch name and enter to confirm or escape to cancel. If i'm not mistaken @windwp accomplished similar thing in the pass. let me check

edit: nvim-telescope/telescope-github.nvim@255dd41

[smithbm2316]

Damn I just noticed it, not sure what's the issue.

I think this action can later be improved to close the prompt after clicking c-a -> create new popup in center of the screen with a message and a field to input branch name and enter to confirm or escape to cancel. If i'm not mistaken @windwp accomplished similar thing in the pass. let me check

edit: nvim-telescope/telescope-github.nvim@255dd41

I definitely like this idea. That implementation may be a bit over my head atm with my current Lua knowledge, but I'm sure with some more practice and tinkering I could make something like that happen. I'll keep that on my radar for a future improvement to this.

[Conni2461]

uhhhhh @smithbm2316 is writing docs :) There is this: #587 also max result query issue still exist but vigoux was looking into that.

Also why did docgen not run for you? Do you have gh actions disabled for that fork?

[smithbm2316]

uhhhhh @smithbm2316 is writing docs :) There is this: #587 also max result query issue still exist but vigoux was looking into that.

Also why did docgen not run for you? Do you have gh actions disabled for that fork?

I was wondering why this didn't work. I'll take a look into this later and see if I can figure out if it's an issue in my fork.