Development and Planning

[ten3roberts]

This discussion serves as a meeting place to discuss and organize development effort for Neogit.

This is primarily focused as a organization tool for maintainers, but you users are equally as free to chime in with good suggestions.

[ten3roberts]

Hey @CKolkey fantastic work on the merge and follow up fixes you've been quick to make.

I've let you pace and handle those mostly as you have the greatest knowledge of a change which may have caused the new bugs, and frankly also because you've been lightning quick about them.

I fully plan to keep working on and improving the functionality of Neogit, and will turn my eyes toward the issues I've assigned to me as I now know I won't cause any merge conflicts with a good couple of open PRs.

If you feel there is a feature or discussion that you don't have in your forked development branch that you'd like someone else to tackle feel free to assign the issue to me, so as to not duplicate efforts on the same feature.

My short term plans moving forward are:

• Rebasing my branch for status refresh timeouts which will fix or at the very least uncover hanging issues, mostly related to refresh lock race conditions
• Checkout commit/revision under the cursor and in the log buffer #556
• Display errors from git nicer (maybe parse them, or reduce the number of required interactions) #537

[ten3roberts]

Will be handling this as well

#550 (comment)

[CKolkey]

What a great influx of interest! So cool. Perfectly timed with my parents/brother flying in to visit 😅

So, to keep up with this growth, I think a couple things should get focused on:

• Test coverage for all popups, as well as unit-tests for the different libs, and.. honestly as much unit testing as possible
• Refactoring the status buffer to be responsible for fewer things. I've been teasing things out of it over the last few months. Again, the focus being to make testing easier.
• I've been reworking how we pick up on changes in the git folder - I have a file-watcher built thats based on how gitsigns does it. This works great during a big rebase, as the status buffer gets updated automatically when it hits a conflict. Hopefully I can get this merged soon, but like I said, family is in town so... might be a minute.
• The help documentation sucks, and really needs to break down what every single popup does, as well as every single action in the popups.

There's more, but thats off the top of my head. I'll add stuff later :)

[CKolkey]

Performance Issues:

• https://github.com/NeogitOrg/neogit/blob/master/lua/neogit/lib/ui/init.lua#L108
  This is an absolutely horrid way to find components, that scales incredibly poorly. For example, in the commit-view buffer, if "context highlight" is enabled in a large buffer, every cursor move can have a 1-2 second delay. Because it's looping over the entire buffer's components every single time.

Ideally, this could probably be done with something like a lookup-table for the top-level ranges (context's), with the table being updated when you open/close folds.

• The remotes popup is a bit laggy to open, due to some of the under the hood cli calls. Make that not laggy.

[ten3roberts]

I agree on all notes. Additionally, perhaps the ui library should be made less magical, as it is very difficult to use goto-definition on the wrappers and function call syntaxes nested within a metatable

[CKolkey]

Public/Private API: #637

We should codify what is public and private.

[ten3roberts]

Yes.

I consider everything not in accessible through require"neogit".exposed_module to not implicitely be part of our public api

[ten3roberts]

I had not considered that the popup change was public api breaking, sorry for that.

I view neogit as having a very slim API surface, as (like the other discussion comparing fugitive) we are less programmatic and aim to provide a interactive UI. Though exposing API's for programmatic commiting etc would be an amazing addition that I am all for, and would make Neogit both a graphical git frontend and a high-level Git library.

[CKolkey]

Yeah, I hadn't either. A non-goal (but something to consider) for this project is to have the buffer/rendering lib be a standalone plugin, and the git lib be a standalone plugin. I don't have any goals of spinning them out, but that level of isolation from the application code is something to aim for, IMO.

I do find it rather funny that (somewhat organically) many of the design choices made in Magit are being made here for... probably similar reasons. Maybe thats not entirely coincidental - I've spent a fair amount of time reading the magit source code - regardless, it's funny.

Allowing for programatic calling of the git libs is a reason, for example, to decouple it from the status buffer, and extract as much from the status buffer, as possible.

As an aside, it's a shame https://github.com/SatyendraBanjare/luagit2 is for lua 5.3 - it would be cool to be able to take advantage of that.

[ten3roberts]

(((yes (I do agree on that sentiment) though I haven't read the source code of magit that much, we are making a very similar plugin and as such we are bound to encounter the same design decisions, and great programmers think alike. )))

( I do really think magit has a splendid setup of the #'magit-commit etc apis (see here 'https://github.com/magit/magit/blob/2126eab6a944e095a65ef00de7cda3bbd0023197/lisp/magit.el#L332))

(I mostly follow magit's user interface and manual and use that as a guideline for making the same observable behavior present in lua)

))))))) ;; In case I forgot any

[ten3roberts]

What do you feel of the builder like git.cli abstraction? I find it quite declarative, but when adding or using git commands you commonly find that the flag or argument you want isn't there, leading you to always need to edit that file. Additionally, it is difficult to know if the function takes an argument, multiple arguments or none since it is all too magical.

In my opinion it would be better to { "git", "remote", "add", remote, url } so that it is much clearer exactly what happens and what will be invoked, and then we build higher level functions around that, like git.remote.add("origin", "<url>") which will provide lsp hints and completion. The current CLI is too much of a black box for my taste

[ten3roberts]

I've learnt that sometimes it is just better to do the thing dirctly and wrap it up in a high level package, rather than abstract all the way down.

Again, I gather lots of influence from Rust, as I've worked with it a tremendous amount and have found many approaches and tricks that work really well, especially in larger system.

[CKolkey]

While I think the current CLI system is quite nifty... you're entirely right. When something is missing, you need to add the flag to a table that uses a metamethod to parse that into a function.. or some functions that return functions that return tables. Like, I think its cool... But it's complex, and since we already have a layer of abstraction over git (git.<module>.<function>()), another layer for the CLI doesn't really add much, since that doesn't (or shouldn't) get used from the application code anyhow.

The biggest advantage that the current builder has is that you can easily do stuff like .files() or .prefix() and they get inserted into the correct positions in the command, and have the requisite -- for files. Thats not a huge thing, but it's nice.

[CKolkey]

I'd be open to scrapping it and simplifying things. Kinda goes hand-in-hand with the planned rewrite of the Process class to use vim.system instead of the vim.fn.job API.

[ten3roberts]

Yes, the files and prefix is nice, though what I've found is that it makes the CLI unpredictable and non-linear as arguments now fly all over the place. And prefix becomes an edge case of before this and after that except not after this, and we end up doing even more when inserting the no-optional-locks prefix-prefix.

I'd prefer if it was predictable in the sense that arguments end up in the order you specify them, rather than giving a special meanings to different argument types that are essentially just a variation of prefixing - -- or -- , but hidden from what it actually does.

I used the vim.system api recently for the testing, and it was really nice and we could definitely make use of it.

[CKolkey]

Alright, I'm all for simplifying things. FWIW gitsigns uses an approach similar to what you're outlining... not similar to what we've got now. And I'm definitely taking inspiration from there, given the quality and overlap of domains.

[CKolkey]

@ten3roberts when you have a sec do you mind adding me as a member of NeogitOrg? Instead of just outside contributor.

[ten3roberts]

Absolutely. Is done now 😊

[CKolkey]

Thanks!

[jarviliam]

Does anyone else have trouble running tests locally? I keep getting module 'tests.util.util' not found on running tests. 🤔

[CKolkey]

Sometimes I need to rm -rf /tmp/neogit and then it all works again. Does.... that fix it? 🧐

[jarviliam]

That doesn't fix it on my end. I noticed that other plugins tend to put the tests folder under the lua folder. After moving it into there and adjusting the run command it works. Should a PR be opened for this? 🤔

[jarviliam]

Heres what I had to change to get it running: master...jarviliam:neogit:fix/test-dir