better auto update logic

6 files changed, 85 insertions, 53 deletions

diff --git a/pkg/gui/confirmation_panel.go b/pkg/gui/confirmation_panel.go
index 17ca62b01..1702b1df6 100644
--- a/pkg/gui/confirmation_panel.go
+++ b/pkg/gui/confirmation_panel.go
@@ -155,6 +155,7 @@ func (gui *Gui) createMessagePanel(g *gocui.Gui, currentView *gocui.View, title,
 }
 
 func (gui *Gui) createErrorPanel(g *gocui.Gui, message string) error {
+	gui.Log.Error(message)
 	currentView := g.CurrentView()
 	colorFunction := color.New(color.FgRed).SprintFunc()
 	coloredMessage := colorFunction(strings.TrimSpace(message))
diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go
index 8783ecba2..48b806ab6 100644
--- a/pkg/gui/gui.go
+++ b/pkg/gui/gui.go
@@ -289,7 +289,7 @@ func (gui *Gui) layout(g *gocui.Gui) error {
 
 		// these are only called once (it's a place to put all the things you want
 		// to happen on startup after the screen is first rendered)
-		gui.Updater.CheckForNewUpdate(gui.onUpdateCheckFinish, false)
+		gui.Updater.CheckForNewUpdate(gui.onBackgroundUpdateCheckFinish, false)
 		gui.handleFileSelect(g, filesView)
 		gui.refreshFiles(g)
 		gui.refreshBranches(g)
@@ -305,46 +305,6 @@ func (gui *Gui) layout(g *gocui.Gui) error {
 	return nil
 }
 
-func (gui *Gui) onUpdateFinish(err error) error {
-	gui.State.Updating = false
-	gui.statusManager.removeStatus("updating")
-	if err := gui.renderString(gui.g, "appStatus", ""); err != nil {
-		return err
-	}
-	if err != nil {
-		return gui.createErrorPanel(gui.g, "Update failed: "+err.Error())
-	}
-	return nil
-}
-
-func (gui *Gui) onUpdateCheckFinish(newVersion string, err error) error {
-	if err != nil {
-		// ignoring the error for now so that I'm not annoying users
-		gui.Log.Error(err.Error())
-		return nil
-	}
-	if newVersion == "" {
-		return nil
-	}
-	if gui.Config.GetUserConfig().Get("update.method") == "background" {
-		gui.startUpdating(newVersion)
-		return nil
-	}
-	title := "New version available!"
-	message := "Download latest version? (enter/esc)"
-	currentView := gui.g.CurrentView()
-	return gui.createConfirmationPanel(gui.g, currentView, title, message, func(g *gocui.Gui, v *gocui.View) error {
-		gui.startUpdating(newVersion)
-		return nil
-	}, nil)
-}
-
-func (gui *Gui) startUpdating(newVersion string) {
-	gui.State.Updating = true
-	gui.statusManager.addWaitingStatus("updating")
-	gui.Updater.Update(newVersion, gui.onUpdateFinish)
-}
-
 func (gui *Gui) fetch(g *gocui.Gui) error {
 	gui.GitCommand.Fetch()
 	gui.refreshStatus(g)
diff --git a/pkg/gui/status_panel.go b/pkg/gui/status_panel.go
index 52842246e..a9dee6a7f 100644
--- a/pkg/gui/status_panel.go
+++ b/pkg/gui/status_panel.go
@@ -50,8 +50,8 @@ func (gui *Gui) renderStatusOptions(g *gocui.Gui) error {
 }
 
 func (gui *Gui) handleCheckForUpdate(g *gocui.Gui, v *gocui.View) error {
-	gui.Updater.CheckForNewUpdate(gui.onUpdateCheckFinish, true)
-	return gui.createMessagePanel(gui.g, v, "Checking for updates", "Checking for updates...")
+	gui.Updater.CheckForNewUpdate(gui.onUserUpdateCheckFinish, true)
+	return gui.createMessagePanel(gui.g, v, "", gui.Tr.SLocalize("CheckingForUpdates"))
 }
 
 func (gui *Gui) handleStatusSelect(g *gocui.Gui, v *gocui.View) error {
diff --git a/pkg/gui/updates.go b/pkg/gui/updates.go
new file mode 100644
index 000000000..0065225fd
--- /dev/null
+++ b/pkg/gui/updates.go
@@ -0,0 +1,57 @@
+package gui
+
+import "github.com/jesseduffield/gocui"
+
+func (gui *Gui) showUpdatePrompt(newVersion string) error {
+	title := "New version available!"
+	message := "Download latest version? (enter/esc)"
+	currentView := gui.g.CurrentView()
+	return gui.createConfirmationPanel(gui.g, currentView, title, message, func(g *gocui.Gui, v *gocui.View) error {
+		gui.startUpdating(newVersion)
+		return nil
+	}, nil)
+}
+
+func (gui *Gui) onUserUpdateCheckFinish(newVersion string, err error) error {
+	if err != nil {
+		return gui.createErrorPanel(gui.g, err.Error())
+	}
+	if newVersion == "" {
+		return gui.createErrorPanel(gui.g, "New version not found")
+	}
+	return gui.showUpdatePrompt(newVersion)
+}
+
+func (gui *Gui) onBackgroundUpdateCheckFinish(newVersion string, err error) error {
+	if err != nil {
+		// ignoring the error for now so that I'm not annoying users
+		gui.Log.Error(err.Error())
+		return nil
+	}
+	if newVersion == "" {
+		return nil
+	}
+	if gui.Config.GetUserConfig().Get("update.method") == "background" {
+		gui.startUpdating(newVersion)
+		return nil
+	}
+	return gui.showUpdatePrompt(newVersion)
+}
+
+func (gui *Gui) startUpdating(newVersion string) {
+	gui.State.Updating = true
+	gui.statusManager.addWaitingStatus("updating")
+	gui.Updater.Update(newVersion, gui.onUpdateFinish)
+}
+
+func (gui *Gui) onUpdateFinish(err error) error {
+	gui.State.Updating = false
+	gui.statusManager.removeStatus("updating")
+	if err := gui.renderString(gui.g, "appStatus", ""); err != nil {
+		return err
+	}
+	if err != nil {
+		return gui.createErrorPanel(gui.g, "Update failed: "+err.Error())
+	}
+	return nil
+}
diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go
index 286d4f7e0..bc51f4136 100644
--- a/pkg/i18n/english.go
+++ b/pkg/i18n/english.go
@@ -315,6 +315,9 @@ func addEnglish(i18nObject *i18n.Bundle) error {
 		}, &i18n.Message{
 			ID:    "CheckForUpdate",
 			Other: "Check for update",
+		}, &i18n.Message{
+			ID:    "CheckingForUpdates",
+			Other: "Checking for updates...",
 		},
 	)
 }
diff --git a/pkg/updates/updates.go b/pkg/updates/updates.go
index 42e77e989..545f6bf83 100644
--- a/pkg/updates/updates.go
+++ b/pkg/updates/updates.go
@@ -2,6 +2,7 @@ package updates
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -81,15 +82,14 @@ func (u *Updater) RecordLastUpdateCheck() error {
 
 // expecting version to be of the form `v12.34.56`
 func (u *Updater) majorVersionDiffers(oldVersion, newVersion string) bool {
+	if oldVersion == "unversioned" {
+		return false
+	}
 	return strings.Split(oldVersion, ".")[0] != strings.Split(newVersion, ".")[0]
 }
 
 func (u *Updater) checkForNewUpdate() (string, error) {
 	u.Log.Info("Checking for an updated version")
-	if u.Config.GetVersion() == "unversioned" {
-		u.Log.Info("Current version is not built from an official release so we won't check for an update")
-		return "", nil
-	}
 	newVersion, err := u.getLatestVersionNumber()
 	if err != nil {
 		return "", err
@@ -102,12 +102,11 @@ func (u *Updater) checkForNewUpdate() (string, error) {
 	}
 
 	if newVersion == u.Config.GetVersion() {
-		return "", nil
+		return "", errors.New("You already have the latest version")
 	}
 
 	if u.majorVersionDiffers(u.Config.GetVersion(), newVersion) {
-		u.Log.Info("New version has non-backwards compatible changes.")
-		return "", nil
+		return "", errors.New("New version has non-backwards compatible changes.")
 	}
 
 	rawUrl, err := u.getBinaryUrl(newVersion)
@@ -116,8 +115,7 @@ func (u *Updater) checkForNewUpdate() (string, error) {
 	}
 	u.Log.Info("Checking for resource at url " + rawUrl)
 	if !u.verifyResourceFound(rawUrl) {
-		u.Log.Error("Resource not found")
-		return "", nil
+		return "", errors.New("Could not find any binary at " + rawUrl)
 	}
 	u.Log.Info("Verified resource is available, ready to update")
 
@@ -142,15 +140,23 @@ func (u *Updater) skipUpdateCheck() bool {
 	// will remove the check for windows after adding a manifest file asking for
 	// the required permissions
 	if runtime.GOOS == "windows" {
+		u.Log.Info("Updating is currently not supported for windows until we can fix permission issues")
+		return true
+	}
+
+	if u.Config.GetVersion() == "unversioned" {
+		u.Log.Info("Current version is not built from an official release so we won't check for an update")
 		return true
 	}
 
 	if u.Config.GetBuildSource() != "buildBinary" {
+		u.Log.Info("Binary is not built with the buildBinary flag so we won't check for an update")
 		return true
 	}
 
 	userConfig := u.Config.GetUserConfig()
 	if userConfig.Get("update.method") == "never" {
+		u.Log.Info("Update method is set to never so we won't check for an update")
 		return true
 	}
 
@@ -158,7 +164,12 @@ func (u *Updater) skipUpdateCheck() bool {
 	lastUpdateCheck := u.Config.GetAppState().LastUpdateCheck
 	days := userConfig.GetInt64("update.days")
 
-	return (currentTimestamp-lastUpdateCheck)/(60*60*24) < days
+	if (currentTimestamp-lastUpdateCheck)/(60*60*24) < days {
+		u.Log.Info("Last update was too recent so we won't check for an update")
+		return true
+	}
+
+	return false
 }
 
 func (u *Updater) mappedOs(os string) string {