jesseduffield / lazygit

simple terminal UI for git commands
MIT License
52.99k stars 1.85k forks source link

Abstract the GUI logic into it's own package #965

Open mjarkk opened 4 years ago

mjarkk commented 4 years ago

Topic Lazygit has become quite complicated over time and to understand the UI logic in the code is also becoming quite a bit more complicated than just understanding GoCui.
If someone wants to add something to lazygit with a tiny bit UI complication they have to understand GoCui and the Lazygit's UI system and i guess that this doesn't help a lot with attracting new developers.
Beside that there is also a point to make about the amount of layers lazygit now has before something is drawn to the terminal (lazygit code > lazygit ui system > gocui > termbox)

Your thoughts To reduce the complexity it would be nice if lazygit had one package like lazyui that does all the UI work so in lazygit there is no code for building the UI only the definition code for the UI.

My second point was just something to think about i don't think it's necessarily needed to be reduced but usually more layers in code means slower programs and more places for errors to occur.

Currently the lazygit UI works but the code gives me the impression from that simpsons dad meme: ![image](https://user-images.githubusercontent.com/15320763/91046399-a4666900-e618-11ea-8f83-1ce2b8caa649.png)
jesseduffield commented 4 years ago

nice meme haha. I agree that we should try and do this, but it is quite the challenge. Definitely something that would make development easier for newcomers

mjarkk commented 4 years ago

Maybe something to think about is testability of the library may this be made.
Lazygit does have a lot of tests but there are still a lot of panics that could have been reduced with some sort of UI tests i imagine.
When we have control over everything that is drawn we can also just keep the layout in memory and add some sort of testing functionality that allows library users to execute function on that layout automatically somewhat like chrome headless or testing in react.

Axel-Jacobsen commented 4 years ago

I think this "Lazyui" idea is great (and I would actually like to use it for another project of mine), and it may be something that I would be willing to give a try. With a quick look at the source code, I found the gui folder - to what extent would "Lazyui" be just plucking that folder from Lazygit and throwing it into it's own repo? FWIW I have not written any Go before, so this may be biting off more than what is reasonable to chew, so to speak.

mjarkk commented 4 years ago

If the primary goal for this was to allow other people to have the same UI as lazygit yes that's mostly what you need to do to get lazygit's UI though that's not necessarily the reason why we would want this.

If someone wants to add something to lazygit with a tiny bit UI complication they have to understand GoCui and the Lazygit's UI system and i guess that this doesn't help a lot with attracting new developers.

The main reason why we would currently want this is to have one UI library in staid of a mix of lazygit's ui and gocui.
Like what gocui does with termbox (the underlaying ui library of gocui) we would have to do that with gocui and that complicates some things.
One way could be to just extract all UI logic from the gui folder like you say but then also make sure there is no need for a gocui import in the project using lazyui.

But as i also state in the original issue:

Beside that there is also a point to make about the amount of layers lazygit now has before something is drawn to the terminal (lazygit code > lazygit ui system > gocui > termbox)

We could get rid of a layer here by removing gocui but that would make this a lot harder than cherry picking the gui folder

Axel-Jacobsen commented 4 years ago

Ahh I see - thanks for the reply! Attempting something like that seems to be a bit out of my scope for right now. I love lazygit, and hope to help out sometime in the future. Keep it up!

jesseduffield commented 2 years ago

I'm doing a refactor of the gui package at the moment that will get us much closer to this