Skip to content

Ability to add custom predefined argument switches to the push menu #1684

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

Open
AlexanderArvidsson opened this issue Mar 14, 2025 · 8 comments · May be fixed by #1705
Open

Ability to add custom predefined argument switches to the push menu #1684

AlexanderArvidsson opened this issue Mar 14, 2025 · 8 comments · May be fixed by #1705

Comments

@AlexanderArvidsson
Copy link
Contributor

AlexanderArvidsson commented Mar 14, 2025

There's a few arguments you can pass while pushing:
Image

It would be great if it was possible to add additional custom options here that can be appended to the push command. One example is git push -o merge_request.create where I would like to be able to add an argument switch on keys -m which would add the argument -o merge_request.create to the push command.

The use-case is once I am done with my feature branch and it's time to make an MR, it's easy to do so automatically via the merge_request.create option.
I believe merge_request.create is a GitLab specific option, hence why my suggested solution is the ability to append custom arguments to the push command, so you can tailor it to whatever backend you use. There's probably other useful options that can be passed via -o <option>.

Right now I have not found any alternatives, I push via the CLI every time I want to make an MR at the same time as the push.

I'd be happy to take a stab at this myself and make an MR, if this is something you guys would be OK with.

@AlexanderArvidsson AlexanderArvidsson changed the title Ability to add custom predefined arguments to the push menu Ability to add custom predefined argument switches to the push menu Mar 14, 2025
@CKolkey
Copy link
Member

CKolkey commented Mar 14, 2025

Hey! Yeah, it would be great to allow users to augment the defaults provided. Though it doesn't need to be specific to the push popup - ideally it's works for any popup that takes arguments.

(As an aside, Q will let you run any git command you like for the current project, but that doesn't mean custom args aren't a good idea).

@AlexanderArvidsson
Copy link
Contributor Author

@CKolkey Agree, the custom arguments would need to be specific for each popup though, as merge_request.create makes no sense for pulls as an example ;)

I'll have a look at crafting a PR with this once I find some time.

@AlexanderArvidsson
Copy link
Contributor Author

AlexanderArvidsson commented Mar 21, 2025

Hi @CKolkey,

I've implemented a working version that I've been using on my own for a bit now, I'm curious if this approach is something you think would work well:

    require('neogit').setup({
      builders = {
        NeogitPushPopup = function(builder)
          --
          builder:switch('m', 'merge_request.create', 'Create merge request', { cli_prefix = '-o ', persisted = false })
        end,
      },
    })

This would allow you to do more than just switches, basically customize the whole menu. Maybe this is too flexible?

I also added a persisted option to switches (default true), as the current approach is a bit awkward with having hardcoded strings in an ignored_settings option which is difficult to extend:

ignored_settings = vim.list_extend({ 'NeogitPushPopup--merge_request.create' }, config.values.ignored_settings)

This could also replace all built-in non-persisted switches (like --force) and eventually possibly remove ignored_settings completely. The name ignored_settings was confusing to say the least, I did not realize it meant to not persist them.

What do you think?

@CKolkey
Copy link
Member

CKolkey commented Mar 21, 2025

I think that looks like a great way to expose the popups to users for customization, and I feel like this builds on top of #1546 very nicely too :)

Also seems natural to add the persisted flag on the switch/option itself. The settings thing is one of the first things I contributed to this project... which is to say theres room for improvement haha.

Let me know when there is something for me to look at!

@AlexanderArvidsson
Copy link
Contributor Author

AlexanderArvidsson commented Mar 21, 2025

One gotcha is since the builder adds switches in the order they're added, all custom ones are always on the end of the list.
Image

Which is not necessarily a bad thing, it would be nice if you could specify an order somehow.
I'm not sure what the best API for that would be. One option would be an order property, and all built-in properties have some default value for this property, like 50, 60, 70, 80. You could then specify values in between to inject your custom options anywhere in the built-in list. This is an arbitrary unit and decimals would work for greater precision (but who's gonna have that many options in reality?).

Or, you could probably simply wipe the switches list and add them in manually on your own in the order you want, for full flexibility?

@CKolkey
Copy link
Member

CKolkey commented Mar 21, 2025

Yeah, thats right. I think for a first-pass it's fine to just append items, but as an extension later the order key with multiples of ten seems reasonable. If you are interjecting more than 9... probably just make a custom popup.

If people want full flexibility they can use the popup api to make their own, so we don't need to worry about that for the built in ones imo. Does that make sense?

AlexanderArvidsson added a commit to AlexanderArvidsson/neogit that referenced this issue Apr 16, 2025
@AlexanderArvidsson
Copy link
Contributor Author

Sorry for the delay here, I've been using it at work for a month now and it has been working so great I kind of forgot about submitting the PR. I did some final cleanup and the PR is at #1705!

@CKolkey
Copy link
Member

CKolkey commented Apr 16, 2025

No worries! I'm glad you took the time to complete this. It looks very nice, i must say.

AlexanderArvidsson added a commit to AlexanderArvidsson/neogit that referenced this issue Apr 19, 2025
CKolkey pushed a commit to AlexanderArvidsson/neogit that referenced this issue Apr 20, 2025
CKolkey pushed a commit to AlexanderArvidsson/neogit that referenced this issue Apr 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants