[WIP] GUI: Rework part 1 #276

glvr182 · 2018-09-10T08:51:55Z

	format	imports	comments	g passing	error checking	error logging
app_status_manager	X	X	X	X	X	X
branches_panel	X	X	X	X	X	X
commit_message_panel	X	X	X	X	X	X
commits_panel	X	X	X	X	X	X
confirmation_panel	X	X	X	X	X	X
files_panel	X	X	X	X	X	X
gui	X	X	X	X	X	X
keybindings	X	X	X	X	X	X
main_panel	X	X	X	X	X	X
menu_panel	X	X	X	X	X	X
merge_panel	X	X	X	X	X	X
stash_panel	X	X	X	X	X	X
status_panel	X	X	X	X	X	X
theme	X	X	X	X	X	X
updates	X	X	X	X	X	X
view_helpers	X	X	X	X	X	X

golangcibot · 2018-09-10T08:52:16Z

pkg/gui/app_status_manager.go

-	name       string
-	statusType string
-	duration   int
+	name       string `json:"name"`


struct field name has json tag but is not exported

golangcibot · 2018-09-10T08:52:16Z

pkg/gui/app_status_manager.go

-	statusType string
-	duration   int
+	name       string `json:"name"`
+	statusType string `json:"status_type"`


struct field statusType has json tag but is not exported

golangcibot · 2018-09-10T08:52:16Z

pkg/gui/app_status_manager.go

-	duration   int
+	name       string `json:"name"`
+	statusType string `json:"status_type"`
+	duration   int    `json:"duration"`


struct field duration has json tag but is not exported

codecov-io · 2018-09-10T08:53:25Z

Codecov Report

Merging #276 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #276      +/-   ##
==========================================
+ Coverage    92.4%   92.42%   +0.01%     
==========================================
  Files           9        9              
  Lines        1553     1557       +4     
==========================================
+ Hits         1435     1439       +4     
  Misses        117      117              
  Partials        1        1

Impacted Files	Coverage Δ
pkg/utils/utils.go	`55.1% <100%> (+3.99%)`	⬆️

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 ca2eec6...fdb0f03. Read the comment docs.

* Removed g parameter * Moved to top * Added error stuff

golangcibot · 2018-09-10T10:42:49Z

pkg/gui/branches_panel.go

+		for _, branch := range gui.State.Branches {
+			fmt.Fprintln(v, branch.GetDisplayString())
+		}
+		gui.resetOrigin(v)


Error return value of gui.resetOrigin is not checked

pkg/gui/branches_panel.go

golangcibot · 2018-09-10T10:42:49Z

pkg/gui/files_panel.go

 			gui.refreshStatus(g)
 		}
-		gui.refreshFiles(g)
+		gui.refreshFiles()


Error return value of gui.refreshFiles is not checked

golangcibot · 2018-09-10T10:42:49Z

pkg/gui/gui.go

 		gui.Updater.CheckForNewUpdate(gui.onBackgroundUpdateCheckFinish, false)
 		gui.handleFileSelect(g, filesView)
-		gui.refreshFiles(g)
+		gui.refreshFiles()


Error return value of gui.refreshFiles is not checked

pkg/gui/gui.go

golangcibot · 2018-09-10T10:42:50Z

pkg/gui/gui.go

 	go func() {
 		for range time.Tick(interval) {
-			function(g)
+			function()


Error return value of function is not checked

pkg/gui/merge_panel.go

golangcibot · 2018-09-10T10:42:50Z

pkg/gui/refresher.go

+		}
+	}
+
+	gui.refreshBranches(gui.g)


Error return value of gui.refreshBranches is not checked

golangcibot · 2018-09-10T10:42:50Z

pkg/gui/refresher.go

+
+	gui.refreshBranches(gui.g)
+	gui.refreshFiles()
+	gui.refreshCommits(gui.g)


Error return value of gui.refreshCommits is not checked

golangcibot · 2018-09-10T12:49:23Z

pkg/gui/files_panel.go

 		} else {
 			gui.closeConfirmationPrompt(g)
-			gui.refreshCommits(g)
+			gui.refreshCommits()


Error return value of gui.refreshCommits is not checked

golangcibot · 2018-09-10T12:49:23Z

pkg/gui/files_panel.go

 			gui.refreshStatus(g)
 		}
-		gui.refreshFiles(g)
+		gui.refreshFiles()


Error return value of gui.refreshFiles is not checked

golangcibot · 2018-09-10T12:49:23Z

pkg/gui/gui.go

-		v.FgColor = gocui.ColorWhite
+		filesView.FgColor = gocui.ColorWhite
+
+		gui.registerRefresher("files", gui.refreshFiles)


Error return value of gui.registerRefresher is not checked

golangcibot · 2018-09-10T12:49:23Z

pkg/gui/gui.go

 		v.Title = gui.Tr.SLocalize("CommitsTitle")
 		v.FgColor = gocui.ColorWhite
+
+		gui.registerRefresher("commits", gui.refreshCommits)


Error return value of gui.registerRefresher is not checked

golangcibot · 2018-09-10T12:49:23Z

pkg/gui/gui.go

-		gui.refreshFiles(g)
-		gui.refreshBranches(g)
-		gui.refreshCommits(g)
+		gui.refreshFiles()


Error return value of gui.refreshFiles is not checked

golangcibot · 2018-09-10T12:49:23Z

pkg/gui/gui.go

-		gui.refreshBranches(g)
-		gui.refreshCommits(g)
+		gui.refreshFiles()
+		gui.refreshBranches()


Error return value of gui.refreshBranches is not checked

golangcibot · 2018-09-10T12:49:23Z

pkg/gui/gui.go

-		gui.refreshCommits(g)
+		gui.refreshFiles()
+		gui.refreshBranches()
+		gui.refreshCommits()


Error return value of gui.refreshCommits is not checked

golangcibot · 2018-09-10T13:13:21Z

pkg/gui/branches_panel.go

+
+			err := gui.GitCommand.Checkout(branch.Name, true)
+			if err != nil {
+				gui.createErrorPanel(g, err.Error())


Error return value of gui.createErrorPanel is not checked

golangcibot · 2018-09-10T13:13:21Z

pkg/gui/branches_panel.go

-		return gui.refreshSidePanels(g)
-	})
+
+	gui.createPromptPanel(g, v, gui.Tr.SLocalize("BranchName")+":",


Error return value of gui.createPromptPanel is not checked

golangcibot · 2018-09-10T13:13:21Z

pkg/gui/files_panel.go

+
+		err := gui.GitCommand.Pull()
+		if err != nil {
+			gui.createErrorPanel(gui.g, err.Error())


Error return value of gui.createErrorPanel is not checked

golangcibot · 2018-09-10T13:13:21Z

pkg/gui/merge_panel.go

 	}
 	gui.stageSelectedFile(g)
-	gui.refreshFiles(g)
+	gui.refreshFiles()


Error return value of gui.refreshFiles is not checked

golangcibot · 2018-09-10T17:17:01Z

pkg/gui/gui.go

+				gui.SubProcess.Stdin = os.Stdin
+				gui.SubProcess.Stdout = os.Stdout
+				gui.SubProcess.Stderr = os.Stderr
+				gui.SubProcess.Run()


Error return value of gui.SubProcess.Run is not checked

Added a lot of if statements Removed view specific render options functions since we now have a global one. Removed a lot of g passing

golangcibot · 2018-09-10T17:58:58Z

pkg/gui/files_panel.go

 	gui.createMessagePanel(g, v, "", gui.Tr.SLocalize("MergeAborted"))
-	gui.refreshStatus(g)
-	return gui.refreshFiles(g)
+	gui.refreshStatus()


Error return value of gui.refreshStatus is not checked

jesseduffield · 2018-09-16T06:17:21Z

pkg/gui/confirmation_panel.go

+		return err
+	}
+
+	err = gui.returnFocus(gui.g, view)


we can remove gui.g from this

Almost done with view_handlers.go then this will be removed

pkg/gui/confirmation_panel.go

pkg/gui/files_panel.go

pkg/gui/gui.go

glvr182 · 2018-09-16T13:18:13Z

@jesseduffield It should all be good now

andreimc

A few points if we want to get this merged in:

Too much whitespace around fns
Don't log on each error just bubble up errors and log at the top
naming is a bit hard to follow I can't understand what vv is in most cases
prefer if err := function(gui.g, v); err != nil { vs more verbose format

Good points:

gui.g is great vs passing g in functions
+1 on the docs

glvr182 · 2018-09-17T09:20:06Z

Seem like fair points, could any of u @jesseduffield @andreimc help me out with changing some thing, kinda busy with work myself atm.

jesseduffield · 2018-09-17T12:57:23Z

Sure I'll jump onto this tomorrow :)

glvr182 · 2018-09-18T09:20:32Z

pkg/gui/branches_panel.go

 // In case something goes wrong it returns an error
 func (gui *Gui) handleBranchPress(g *gocui.Gui, v *gocui.View) error {
-	v, err := gui.g.View("branches")
+	branchesView, err := gui.g.View("branches")


I think v should be used here instead since it will always be the branches view

hmm am I right in thinking we can just use the v that gets passed in?

Yes, since the menu special case got solved, this v will always be branches

jesseduffield · 2018-09-18T09:20:43Z

I pushed a commit that has aligned a few panels (files panel, branches panel, commits panel, and a couple others iirc) with the structure I'm looking for

jesseduffield · 2018-09-18T09:23:16Z

also I've noticed that the application is slow to quit on this branch. I'm not sure what might be causing that

glvr182 · 2018-09-18T09:28:59Z

I dont seem to have that problem, how bad is it

jesseduffield · 2018-09-18T09:34:46Z

about 2 seconds of latency

glvr182 · 2018-12-09T14:17:17Z

Going to break this up into smaller pr's

STATUS: Cleaned, formatted, commented and changed performance

d544632

golangcibot reviewed Sep 10, 2018

View reviewed changes

STATUS: Removed unused json naming

47d526d

glvr182 added 10 commits September 10, 2018 11:08

MAIN: Moved main to its own file

545886d

BRANCHES: Moved refresh function to top

7886af9

GUI: Moved refresh function to own file

d397084

FILES: Changed refreshFiles

0af5d1a

* Removed g parameter * Moved to top * Added error stuff

BRANCHES: Refactor to new standard

d2c9831

STASH: Refactor to new standard

5ba8b5e

MERGE: Refactor to new standard

e6a1fce

COMMIT: Add error stuff

4010399

COMMIT: Refactor to new standard

0ece02a

GUI: Refactor to new standard

6c3fc5f

golangcibot reviewed Sep 10, 2018

View reviewed changes

glvr182 added 2 commits September 10, 2018 14:47

REFRESH: Made the refresher work

bca40a1

REFRESH: Start using refresher across files

e534015

golangcibot reviewed Sep 10, 2018

View reviewed changes

GUI: More error checking

c1d4059

golangcibot reviewed Sep 10, 2018

View reviewed changes

glvr182 added 4 commits September 10, 2018 17:06

GUI: Error checking

4414846

TEST: Added test for max

df94df3

GUI: Added error checking and removed g passing

ec7aca7

GUI: Moved some functions, added error handling, less g passing

d0edf9d

golangcibot reviewed Sep 10, 2018

View reviewed changes

glvr182 added 3 commits September 10, 2018 19:26

GUI: Some more error handling, cleaning and moving

5a43de2

GUI: More error handling, formatting and removing unneeded functions

db59ae0

Added a lot of if statements Removed view specific render options functions since we now have a global one. Removed a lot of g passing

GUI: Removed g passing

42db17a

golangcibot reviewed Sep 10, 2018

View reviewed changes

FILES: Changed .Error to .Errorf and added error checking to functions

9359a36

jesseduffield reviewed Sep 16, 2018

View reviewed changes

pkg/gui/confirmation_panel.go Outdated Show resolved Hide resolved

jesseduffield reviewed Sep 16, 2018

View reviewed changes

pkg/gui/files_panel.go Outdated Show resolved Hide resolved

jesseduffield reviewed Sep 16, 2018

View reviewed changes

pkg/gui/files_panel.go Outdated Show resolved Hide resolved

jesseduffield reviewed Sep 16, 2018

View reviewed changes

pkg/gui/gui.go Show resolved Hide resolved

jesseduffield reviewed Sep 16, 2018

View reviewed changes

pkg/gui/gui.go Outdated Show resolved Hide resolved

glvr182 and others added 11 commits September 16, 2018 14:26

VIEWS: Formatting, error checking, error logging, imports, g passing

6249d5b

VIEW: Making ci happy

0ecbd39

GUI: Removed whitespaces at start of function

b4c1fcf

BRANCHES: Removed whitespace

95cc902

CONFIRMATION: whitespaces

6089589

FILES: Removed unneeded var declaration

fa5bc4d

FILES: Added missing error log, removed forgotten g passing

5fac95b

GUI: Added pani

f1a6e41

VIEW: Added missing args to logging

4c4caf6

Merge branch 'master' into feature/rework-gui-pkg

9bf4f8d

COMMIT_MESSAGE: Fixed format

fe874e1

andreimc self-requested a review September 17, 2018 01:48

andreimc suggested changes Sep 17, 2018

View reviewed changes

compacting functions

fdb0f03

glvr182 commented Sep 18, 2018

View reviewed changes

glvr182 closed this Dec 9, 2018

Uh oh!

[WIP] GUI: Rework part 1 #276

[WIP] GUI: Rework part 1 #276

Uh oh!

Conversation

glvr182 commented Sep 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

codecov-io commented Sep 10, 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!

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!

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!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

glvr182 commented Sep 16, 2018

Uh oh!

andreimc left a comment

Choose a reason for hiding this comment

Uh oh!

glvr182 commented Sep 17, 2018

Uh oh!

jesseduffield commented Sep 17, 2018

Uh oh!

glvr182 Sep 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

glvr182 commented Sep 10, 2018 •

edited

Loading

codecov-io commented Sep 10, 2018 •

edited

Loading

glvr182 Sep 18, 2018 •

edited

Loading