add streaming git command status panel #296

scbizu · 2018-09-20T06:01:47Z

golangcibot · 2018-09-20T06:01:58Z

pkg/gui/command_status_panel.go

+const (
+	// cmdStatusLen is a magic number
+	// that defines the max columns of CommandStatus panel
+	cmdStatusLen = 12


cmdStatusLen is unused

jesseduffield · 2018-09-20T06:54:57Z

Very good idea! This looks like a very promising PR

ponsfrilus · 2018-09-20T06:56:18Z

[off topic] @scbizu ingress enlightened spotted :)

codecov-io · 2018-09-28T08:19:24Z

Codecov Report

Merging #296 into master will increase coverage by 0.09%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #296      +/-   ##
==========================================
+ Coverage   84.89%   84.98%   +0.09%     
==========================================
  Files          21       21              
  Lines        3515     3537      +22     
==========================================
+ Hits         2984     3006      +22     
  Misses        491      491              
  Partials       40       40

Impacted Files	Coverage Δ
pkg/commands/git.go	`65.53% <100%> (+0.41%)`	⬆️
pkg/i18n/english.go	`100% <100%> (ø)`	⬆️
pkg/i18n/dutch.go	`100% <100%> (ø)`	⬆️
pkg/i18n/polish.go	`100% <100%> (ø)`	⬆️
pkg/commands/os.go	`73.33% <100%> (+1.3%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 471733f...d5daf9b. Read the comment docs.

golangcibot · 2018-10-05T12:54:28Z

pkg/commands/git_test.go

+func TestGitCommandCommitWithStatus(t *testing.T) {
+	type scenario struct {
+		testName           string
+		command            func(string, ...string) *cmd.Cmd


command is unused

jesseduffield · 2018-10-10T06:59:27Z

pkg/gui/command_status_panel.go

+	go func() {
+		for {
+			select {
+			case status := <-cs.command.Stdout:


what would be an example value for status? Would it be a random string read from the buffer, or a single character etc?

also I'm not sure if this is possible but can we call cs.refreshCommandStatus(cs.gui, "") after the command has completed so that we have line breaks between the outputs of different commands (utilising the newline added in refreshCommandStatus)?

The value will be the plain git command output, you can check the output within the dev mode by grep collect\ message,however, I will rename it with the cmdOutput for better understanding .

Do you mean add new line break for different git command? To implement this, I think we'd better to pass a echo \n(shell) to command status panel.

yes, that's what I mean. Although we'd still need to know when to pass that command, which would require us to know when the first command has finished, which if we knew, we could just add a newline. I'm happy to leave this issue as too-tricky-to-solve for now, unless you have an idea of how to do it :)

Yeah. we can receive the done singnal when the cmd is done. And then we add this line break.

jesseduffield · 2018-10-10T06:59:51Z

pkg/gui/command_status_panel.go

+}
+
+// Update updates the command status panel
+func (cs *CommandStatus) Update(ui *Gui) {


conventionally we use gui as the variable name for *Gui, rather than ui

pkg/gui/command_status_panel.go

jesseduffield · 2018-10-10T07:01:48Z

pkg/gui/commit_message_panel.go

+
+	c, isGPG := gui.GitCommand.CommitWithStatus(message)
+	if isGPG {
+		// put it into subprogress


minor typo: this should read subprocess rather than subprogress

jesseduffield · 2018-10-10T07:03:00Z

pkg/gui/commit_message_panel.go

+		// put it into subprogress
+		sub, err := gui.GitCommand.Commit(message)
+		if err != nil {
+			// TODO need to find a way to send through this error


is this TODO still necessary? using an error panel should be fine in my opinion

I am not sure whether I can remove this, so I just keep it as before.

jesseduffield · 2018-10-10T07:05:19Z

pkg/gui/command_status_panel.go

+}
+
+// Update updates the command status panel
+func (cs *CommandStatus) Update(ui *Gui) {


I think this could have a better name, something like PrintCommandOutput. The name Update suggests to me that it will be an immediate operation

jesseduffield · 2018-10-10T07:13:42Z

pkg/gui/gui.go

@@ -136,6 +142,17 @@ func (gui *Gui) scrollDownMain(g *gocui.Gui, v *gocui.View) error {
 	return nil
 }

+func (gui *Gui) scrollDownCommandStatus(g *gocui.Gui, v *gocui.View) error {


this duplicates some code from the above scroll method. Could we make a generic function that takes a view name and returns a method for scrolling that view? Something like this:

func (gui *Gui) genericScrollDown(viewName string) func(g *gocui.Gui, v *gocui.View) error { return func (g *gocui.Gui, v *gocui.View) error { view, err := g.View(viewName) if err != nil { return err } ox, oy := view.Origin() if oy < len(view.BufferLines()) { return view.SetOrigin(ox, oy+gui.Config.GetUserConfig().GetInt("gui.scrollHeight")) } return nil } }

then in our keybindings file we can use it like so:

}, { ViewName: "", Key: gocui.KeyPgdn, Modifier: gocui.ModNone, Handler: gui.genericScrollDown("main"), }, {

jesseduffield · 2018-10-10T07:15:28Z

pkg/gui/gui.go

-			return err
+	isStreamingStatusEnable := gui.Config.GetUserConfig().GetBool("commandStatus")
+
+	if isStreamingStatusEnable {


this duplicates some code. The only difference between the two blocks in the if statement is commitsBranchesBoundary vs optionsTop, so we can just assign that value to a variable based on the value of isStreamingStatusEnable and then we don't need to have the SetView method repeated

jesseduffield · 2018-10-10T07:16:11Z

pkg/gui/gui.go

@@ -217,7 +248,7 @@ func (gui *Gui) layout(g *gocui.Gui) error {
 		}
 		filesView.Highlight = true
 		filesView.Title = gui.Tr.SLocalize("FilesTitle")
-		v.FgColor = gocui.ColorWhite
+		filesView.FgColor = gocui.ColorWhite


good pickup ;)

jesseduffield · 2018-10-10T07:18:33Z

pkg/gui/gui.go

@@ -429,6 +471,13 @@ func (gui *Gui) quit(g *gocui.Gui, v *gocui.View) error {
 		return gui.createUpdateQuitConfirmation(g, v)
 	}
 	if gui.Config.GetUserConfig().GetBool("confirmOnQuit") {
+		// clear command status buffer


I'm not sure why we're clearing this buffer if we're about to quit anyway

It is seems unnecessary to do this XD

jesseduffield

Cool stuff! I've been using it throughout the week and it seems pretty stable.

I left some comments :)

scbizu · 2018-10-12T02:34:48Z

Thanks for your advice , i will do some code fix this weekend.

…ptimization

commands: add commit status flags gui: fix commit status gui: fix streaming panel blocking

scbizu · 2019-04-17T18:01:29Z

Hello @jesseduffield , I rebased my code, and pushed again for re-review.

(BTW, The package management is the really disappointing thing , when I add my cmd repo in toml, and run dep ensure , the vendor comes with so many changes XD. Really MISSING the go mod now~

mjarkk · 2019-05-03T07:42:28Z

@scbizu I have a number of questions and comments..

How can that this pull adds ~3,300,000 extra lines of code?
The go.mod file is removed in master
I can be wrong here but i can remember from looking thought your code that there was a PTY package somewhere. PTYs don't work on the windows subsystem see: https://github.com/kr/pty/issues/67 , If you have no access to windows i'm fine with testing this for you on win

scbizu · 2019-05-05T02:53:59Z

@mjarkk Hi, thanks for your reply.

Because of the missing version tag in Gopkg.toml(I mean the constraint),when I run dep ensureto add another dependency, the out-of-date package will update to its new release.
I forget to push my recent commit XD , thanks! (go mod is removed)
In master Gopkg.toml,the pty package points to github.com/jesseduffield/pty? According to the CI report, I do not find any breaks for Windows builds.

mjarkk · 2019-05-05T09:27:41Z

In master Gopkg.toml,the pty package points to github.com/jesseduffield/pty? According to the CI report, I do not find any breaks for Windows builds.

Great to hear that the program still work on win :),
The jesseduffield/pty package is a problem but that has nothing to do with your PR
There is also already a PR open to fix the usage of that package.

scbizu · 2019-05-05T09:41:07Z

In master Gopkg.toml,the pty package points to github.com/jesseduffield/pty? According to the CI report, I do not find any breaks for Windows builds.

Great to hear that the program still work on win :),
The jesseduffield/pty package is a problem but that has nothing to do with your PR
There is also already a PR open to fix the usage of that package.

Sounds GREAT XD

jesseduffield · 2019-05-06T11:13:35Z

Giving this a quick local test, I've noticed a few things:

The CommandStatus only has output when I'm doing a git commit. Is this expected behaviour? Or do we also want this printing a line whenever we run any git (or bash) command?
After committing a file, the files view is not updated immediately, meaning it is only refreshed to show that the staged file is no longer present after the automatic refresh that happens every ten seconds
The output has no colour, which makes pre-commit output hard to read if it is usually coloured. I'm not sure if there's a way around this.
The CommandStatus panel does not scroll automatically once the content spills out of view
It would be nice to be able to scroll the commandStatus view with the mouse, as you can do with the other panels.

scbizu · 2019-05-07T03:15:17Z

Giving this a quick local test, I've noticed a few things:

The CommandStatus only has output when I'm doing a git commit. Is this expected behaviour? Or do we also want this printing a line whenever we run any git (or bash) command?

After committing a file, the files view is not updated immediately, meaning it is only refreshed to show that the staged file is no longer present after the automatic refresh that happens every ten seconds

The output has no colour, which makes pre-commit output hard to read if it is usually coloured. I'm not sure if there's a way around this.

The CommandStatus panel does not scroll automatically once the content spills out of view

It would be nice to be able to scroll the commandStatus view with the mouse, as you can do with the other panels.

yep, my original idea is to fix the pre-commit message. So, hmm, these changes currently not support other git commands. Others will be easily integrated once the streaming panel works as we excepted.
Is this a known issue or the new issue 🙈 ?
The streaming panel only forwards the stdout and stderr. According to my knowledge, it will keep the color text. But I haven't test it yet,I will give it a try these days (sounds like an interesting point ;) ).
Great point, I will support this these days.
Mouse... I will try it .

jesseduffield · 2020-03-23T12:25:38Z

doing a cleanup of PR's so I'm closing this but if you ever want to give it another shot feel free to raise another PR :)

marcjay · 2020-05-12T19:11:18Z

This PR was looking promising - I'm looking to see the output from a git push command as that includes a link to raise a PR, e.g.:

remote: Create pull request for <branch-name>:
remote:   https://<URL-to-create-PR-here>

Am aware of the bitbucket/github integration but the version of bitbucket I'm using uses a different URL structure. Was this PR addressing push output as well, or just commit? Wanting to know whether to raise a separate issue for this.
I suspect the other option is to raise an issue/PR for extending the 'create pull request' support to have configurable URLs

scbizu · 2020-05-13T05:50:27Z

Hi @matejcik , this PR does nothing special but only output which git push(or other git commands) does , which means that if your git command shows you

remote: Create pull request for <branch-name>:
remote:   https://<URL-to-create-PR-here>

and then the streaming panel will be too .

(Since I unfollowed the lazygit ecosystem too long so that maybe this PR is already out-of-date and has no guaranteed deadline)

golangcibot reviewed Sep 20, 2018

View reviewed changes

scbizu mentioned this pull request Sep 20, 2018

lazygit do not support git pre commit #92

Open

scbizu force-pushed the feature/go-cmd branch 7 times, most recently from 3cdb39e to c6601b5 Compare September 28, 2018 08:14

golangcibot reviewed Oct 5, 2018

View reviewed changes

scbizu force-pushed the feature/go-cmd branch 3 times, most recently from 3cc7347 to 96f2b36 Compare October 5, 2018 14:06

scbizu changed the title ~~WIP: add streaming git command status panel~~ add streaming git command status panel Oct 6, 2018

jesseduffield reviewed Oct 10, 2018

View reviewed changes

pkg/gui/command_status_panel.go Show resolved Hide resolved

jesseduffield reviewed Oct 10, 2018

View reviewed changes

jesseduffield requested changes Oct 10, 2018

View reviewed changes

scbizu force-pushed the feature/go-cmd branch from 0409387 to 77680d8 Compare October 13, 2018 06:13

scbizu added 15 commits April 17, 2019 14:17

commands: remove gpg streaming status code

db4f9b6

gui: remove CmdStatus

8abe719

gui: use a variable to detect if the commandStatus is set.

ec616ee

command: fix up the streaming status expected testing res

9da2f13

add streaming panel readme

3963d31

pkg/gui: rename variables for better understanding and do some code o…

1dd9f6e

…ptimization

pkg/gui: fix mirror typo

c75fd57

gui: add a common scroll down/up solution

6748bba

gui: replace with the common scroll solution

f1d6be5

gui: remove duplicate code

1961da3

gui: resolve git commit with non-amend mode

a60f863

gui: fix channel block

1328f96

gui: fix common scroll up/down

4754740

dep: update dependency

dd3b693

gui: fix unknown main view

6bfd692

commands: add commit status flags gui: fix commit status gui: fix streaming panel blocking

scbizu force-pushed the feature/go-cmd branch from 5e4b513 to 6bfd692 Compare April 17, 2019 17:25

scbizu added 3 commits April 18, 2019 01:28

commands: fix cmd testing

3ec3f66

dep: update dep

99db07a

mod: remove go mod

d5daf9b

jesseduffield closed this Mar 23, 2020

Uh oh!

add streaming git command status panel #296

add streaming git command status panel #296

Uh oh!

Conversation

scbizu commented Sep 20, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jesseduffield commented Sep 20, 2018

Uh oh!

ponsfrilus commented Sep 20, 2018

Uh oh!

codecov-io commented Sep 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jesseduffield left a comment

Choose a reason for hiding this comment

Uh oh!

scbizu commented Oct 12, 2018

Uh oh!

scbizu commented Apr 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjarkk commented May 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scbizu commented May 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjarkk commented May 5, 2019

Uh oh!

scbizu commented May 5, 2019

Uh oh!

jesseduffield commented May 6, 2019

Uh oh!

scbizu commented May 7, 2019

Uh oh!

jesseduffield commented Mar 23, 2020

Uh oh!

marcjay commented May 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scbizu commented May 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Sep 28, 2018 •

edited

Loading

scbizu commented Apr 17, 2019 •

edited

Loading

mjarkk commented May 3, 2019 •

edited

Loading

scbizu commented May 5, 2019 •

edited

Loading

marcjay commented May 12, 2020 •

edited

Loading

scbizu commented May 13, 2020 •

edited

Loading