Add specific error messages to GitServerConfigActivity

3 files changed, 44 insertions, 27 deletions

diff --git a/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt b/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt
index 591a8581..8ad495fa 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt
@@ -17,6 +17,7 @@ import androidx.preference.PreferenceManager
 import com.github.ajalt.timberkt.Timber.tag
 import com.github.ajalt.timberkt.e
 import com.google.android.material.dialog.MaterialAlertDialogBuilder
+import com.zeapo.pwdstore.R
 import com.zeapo.pwdstore.git.config.ConnectionMode
 import com.zeapo.pwdstore.git.config.Protocol
 import com.zeapo.pwdstore.git.config.SshApiSessionFactory
@@ -83,14 +84,25 @@ abstract class BaseGitActivity : AppCompatActivity() {
         super.onDestroy()
     }
 
+    enum class GitUpdateUrlResult(val textRes: Int) {
+        Ok(0),
+        CustomPortRequiresAbsoluteUrlError(R.string.git_config_error_custom_port_absolute),
+        EmptyHostnameError(R.string.git_config_error_hostname_empty),
+        GenericError(R.string.git_config_error_generic),
+        NonNumericPortError(R.string.git_config_error_nonnumeric_port)
+    }
+
     /**
-     * Update the [url] field with the values that build it up. This function returns a boolean
-     * indicating whether or not the values are likely valid or not, and only adds the `origin`
-     * remote when it is. This check is not perfect, it is mostly meant to catch typos.
+     * Update the [url] field with the values that build it up. This function returns a
+     * [GitUpdateUrlResult] indicating whether the values could be used to build a URL and only adds
+     * the `origin` remote when they were. This check is not perfect, it is mostly meant to catch
+     * syntax-related typos.
      */
-    fun updateUrl(): Boolean {
-        if (serverHostname.isEmpty() || !serverPort.isDigitsOnly())
-            return false
+    fun updateUrl(): GitUpdateUrlResult {
+        if (serverHostname.isEmpty())
+            return GitUpdateUrlResult.EmptyHostnameError
+        if (!serverPort.isDigitsOnly())
+            return GitUpdateUrlResult.NonNumericPortError
 
         val previousUrl = url ?: ""
         // Whether we need the leading ssh:// depends on the use of a custom port.
@@ -105,9 +117,9 @@ abstract class BaseGitActivity : AppCompatActivity() {
                     val pathPart = serverPath.trimStart('/', ':')
                     "$userPart$hostnamePart:$pathPart"
                 } else {
-                    // We only support absolute paths with custom ports.
+                    // Only absolute paths are supported with custom ports.
                     if (!serverPath.startsWith('/'))
-                        return false
+                        return GitUpdateUrlResult.CustomPortRequiresAbsoluteUrlError
                     val pathPart = serverPath
                     // We have to specify the ssh scheme as this is the only way to pass a custom
                     // port.
@@ -128,9 +140,9 @@ abstract class BaseGitActivity : AppCompatActivity() {
                     if (URI(url).rawAuthority != null)
                         url
                     else
-                        return false
+                        return GitUpdateUrlResult.GenericError
                 } catch (_: Exception) {
-                    return false
+                    return GitUpdateUrlResult.GenericError
                 }
             }
         }
@@ -141,7 +153,7 @@ abstract class BaseGitActivity : AppCompatActivity() {
         if (previousUrl.isNotEmpty() && newUrl != previousUrl && protocol == Protocol.Https)
             encryptedSettings.edit { remove("https_password") }
         url = newUrl
-        return true
+        return GitUpdateUrlResult.Ok
     }
 
     /**
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/GitServerConfigActivity.kt b/app/src/main/java/com/zeapo/pwdstore/git/GitServerConfigActivity.kt
index 3e5d18e0..f8c67e51 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/GitServerConfigActivity.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/GitServerConfigActivity.kt
@@ -97,22 +97,23 @@ class GitServerConfigActivity : BaseGitActivity() {
         binding.saveButton.setOnClickListener {
             if (isClone && PasswordRepository.getRepository(null) == null)
                 PasswordRepository.initialize(this)
-            if (updateUrl()) {
-                settings.edit {
-                    putString("git_remote_protocol", protocol.pref)
-                    putString("git_remote_auth", connectionMode.pref)
-                    putString("git_remote_server", serverHostname)
-                    putString("git_remote_port", serverPort)
-                    putString("git_remote_username", serverUser)
-                    putString("git_remote_location", serverPath)
+            when (val result = updateUrl()) {
+                GitUpdateUrlResult.Ok -> {
+                    settings.edit {
+                        putString("git_remote_protocol", protocol.pref)
+                        putString("git_remote_auth", connectionMode.pref)
+                        putString("git_remote_server", serverHostname)
+                        putString("git_remote_port", serverPort)
+                        putString("git_remote_username", serverUser)
+                        putString("git_remote_location", serverPath)
+                    }
+                    if (!isClone) {
+                        Snackbar.make(binding.root, getString(R.string.git_server_config_save_success), Snackbar.LENGTH_SHORT).show()
+                        Handler().postDelayed(500) { finish() }
+                    } else
+                        cloneRepository()
                 }
-                if (!isClone) {
-                    Snackbar.make(binding.root, getString(R.string.git_server_config_save_success), Snackbar.LENGTH_SHORT).show()
-                    Handler().postDelayed(500) { finish() }
-                } else
-                    cloneRepository()
-            } else {
-                Snackbar.make(binding.root, getString(R.string.git_server_config_save_failure), Snackbar.LENGTH_LONG).show()
+                else -> Snackbar.make(binding.root, getString(R.string.git_server_config_save_error_prefix, getString(result.textRes)), Snackbar.LENGTH_LONG).show()
             }
         }
     }
diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
index 2c2d4d55..1b2f9c52 100644
--- a/app/src/main/res/values/strings.xml
+++ b/app/src/main/res/values/strings.xml
@@ -363,7 +363,11 @@
     <string name="connection_mode_openkeychain" translatable="false">OpenKeychain</string>
     <string name="connection_mode_none">None</string>
     <string name="git_server_config_save_success">Successfully saved configuration</string>
-    <string name="git_server_config_save_failure">Configuration error: please verify your settings and try again</string>
+    <string name="git_server_config_save_error_prefix">Configuration error: %s</string>
+    <string name="git_config_error_hostname_empty">empty hostname</string>
+    <string name="git_config_error_generic">please verify your settings and try again</string>
+    <string name="git_config_error_nonnumeric_port">port must be numeric</string>
+    <string name="git_config_error_custom_port_absolute">path must be absolute (start with \'/\') when using a custom port</string>
     <string name="git_operation_unable_to_open_ssh_key_title">Unable to open the ssh-key</string>
     <string name="git_operation_unable_to_open_ssh_key_message">Please check that it was imported.</string>
     <string name="git_operation_wrong_passphrase">Wrong passphrase</string>