Skip to content
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

Clean up annotations on APIs that can be made public and document the ability to create custom popups #1546

Merged
merged 6 commits into from
Nov 7, 2024

Conversation

zivarah
Copy link
Contributor

@zivarah zivarah commented Nov 6, 2024

As noted in #1542, it could be very useful to use Neogit as a platform for building popups and actions for custom subcommands. Clean up some of the documentation around the builder APIs and mention in the docs the possibility of creating custom commands.

doc/neogit.txt Show resolved Hide resolved
@CKolkey
Copy link
Member

CKolkey commented Nov 6, 2024

Not going to lie, this is actually way simpler than I had in mind - in a good way. I had this whole idea about replacing the builder pattern with a completely declarative approach, which would be a lot of work and, as this shows... entirely not needed. Thanks for that :)

Question for you: would making the git.* modules public be useful? Or a subset of them? Or leave it up to the user to get data via vim.system() or whatever they like. Definitely not something for this PR if so. Just wondering how much of the internal infrastructure would be valuable.

Another aspect of this that would be good to show would be how to integrate a custom popup with the status buffer:

  • How the selection gets passed into the popup constructor function
  • How to trigger a refresh of the status buffer after running an action

I think those are somewhat unsolved by this (which is fine) - but again, might not be a big deal. What do you think?

@zivarah
Copy link
Contributor Author

zivarah commented Nov 6, 2024

Not going to lie, this is actually way simpler than I had in mind - in a good way. I had this whole idea about replacing the builder pattern with a completely declarative approach, which would be a lot of work and, as this shows... entirely not needed. Thanks for that :)

Oh interesting! Yeah, I figured that the existing mechanisms were fine as they are.

Question for you: would making the git.* modules public be useful? Or a subset of them? Or leave it up to the user to get data via vim.system() or whatever they like. Definitely not something for this PR if so. Just wondering how much of the internal infrastructure would be valuable.

It seems that would be fine -- I haven't really used them but from a glance they seem easy enough to use. The documentation is a bit sparse for some of them, but I think that could be fleshed out in the future.

Another aspect of this that would be good to show would be how to integrate a custom popup with the status buffer:

  • How the selection gets passed into the popup constructor function
  • How to trigger a refresh of the status buffer after running an action

I think those are somewhat unsolved by this (which is fine) - but again, might not be a big deal. What do you think?

I'm less familiar with these -- would you mind tweaking what I have pushed currently to include what you're envisioning for that? Happy to include it!

@CKolkey
Copy link
Member

CKolkey commented Nov 7, 2024

It seems that would be fine -- I haven't really used them but from a glance they seem easy enough to use. The documentation is a bit sparse for some of them, but I think that could be fleshed out in the future.

Heh, yeah, I was learning lua for the most part by working on this in the beginning. Had no idea how to do type annotations (and they weren't nearly as ubiquitous two years ago, iirc). I try to go back over them to add better types/docs or whatever from time to time.

Another aspect of this that would be good to show would be how to integrate a custom popup with the status buffer:

  • How the selection gets passed into the popup constructor function
  • How to trigger a refresh of the status buffer after running an action

I think those are somewhat unsolved by this (which is fine) - but again, might not be a big deal. What do you think?

I'm less familiar with these -- would you mind tweaking what I have pushed currently to include what you're envisioning for that? Happy to include it!

I think I can make the latter automatic, and add better docs/types for the former.

@CKolkey CKolkey merged commit a9c6864 into NeogitOrg:master Nov 7, 2024
5 checks passed
@CKolkey
Copy link
Member

CKolkey commented Nov 7, 2024

Really appreciate your initiative here - thank you!

@zivarah zivarah deleted the custom-popups branch November 7, 2024 14:16
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 this pull request may close these issues.

2 participants