Fix issues around the property config panel (#142). There were lots o…

…f little things wrong with it, and one big critical issue.
groboclown · Feb 28, 2017 · 81d61ee · 81d61ee
1 parent 0edd225
commit 81d61ee
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 6 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -12,6 +12,12 @@
 * Bug fixes.
     * The relative config part of the configuration no longer hangs when
       the configuration is first loaded (#139).
+    * The property configuration did not correctly write its parameters to the
+      `workspace.xml` file (#142).
+    * The property configuration did not correctly report itself as not
+      modified.
+    * The property configuration pane didn't let you choose the file by
+      pressing the "..." button.
 
 
 ## ::v0.8.2::

diff --git a/plugin/META-INF/plugin.xml b/plugin/META-INF/plugin.xml
@@ -9,7 +9,13 @@
           <li><em>0.8.3</em>
             <ol>
               <li>The relative config part of the configuration no longer hangs when
-              the configuration is first loaded</li>
+              the configuration is first loaded.</li>
+              <li>The property configuration did not correctly write its parameters to the
+              <tt>workspace.xml</tt> file.</li>
+              <li>The property configuration did not correctly report itself as not
+              modified.</li>
+              <li>The property configuration pane didn't let you choose the file by
+              pressing the "..." button.</li>
             </ol>
           </li>
           <li><em>0.8.2</em>

diff --git a/plugin/src/net/groboclown/idea/p4ic/P4Bundle.properties b/plugin/src/net/groboclown/idea/p4ic/P4Bundle.properties
@@ -451,4 +451,10 @@ error.authorization.retry=Failed to authenticate after too many retries.
 configuration.resolve.key.server-fingerprint=Server Fingerprint
 configuration.resolve.password.empty=(Password empty)
 statusbar.connection.popup.reload=Reload connections
-statusbar.connection.popup.show-config=Show configuration
+statusbar.connection.popup.show-config=Show configuration
+configuration.properties.authticket.chooser.title=Select Authentication File
+configuration.properties.authticket.chooser.desc=Select the file that stores the Perforce server login token.  Leave blank to force a password.
+configuration.properties.trustticket.chooser.title=Select Trust Ticket  File
+configuration.properties.trustticket.chooser.desc=Select the file that stores the trusted server information.  Used with SSL connections.
+configuration.properties.loginsso.chooser.title=Select Login SSO Executable
+configuration.properties.loginsso.chooser.desc=Select the executable that performs the Perforce signle sign on.
diff --git a/plugin/src/net/groboclown/idea/p4ic/config/part/PartValidation.java b/plugin/src/net/groboclown/idea/p4ic/config/part/PartValidation.java
@@ -62,7 +62,8 @@ boolean checkPort(@NotNull DataPart part, @Nullable String rawPort) {
     }
 
     boolean checkAuthTicketFile(@NotNull ConfigPart part, @Nullable File file) {
-        if (file != null && (! file.exists() || ! file.isFile())) {
+        // If it points to a directory, then we ignore this.
+        if (file != null && ! file.exists()) {
             problems.add(new ConfigProblem(part, "configuration.problem.authticket.exist", file));
             return false;
         }
@@ -74,7 +75,8 @@ boolean checkAuthTicketFile(@NotNull DataPart part) {
     }
 
     boolean checkTrustTicketFile(@NotNull ConfigPart part, @Nullable File file) {
-        if (file != null && (! file.exists() || ! file.isFile())) {
+        // If it points to a directory, then we ignore this.
+        if (file != null && ! file.exists()) {
             problems.add(new ConfigProblem(part, "configuration.problem.trustticket.exist", file));
             return false;
         }

diff --git a/plugin/src/net/groboclown/idea/p4ic/config/part/SimpleDataPart.java b/plugin/src/net/groboclown/idea/p4ic/config/part/SimpleDataPart.java
@@ -14,6 +14,7 @@
 
 package net.groboclown.idea.p4ic.config.part;
 
+import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.vfs.VirtualFile;
 import net.groboclown.idea.p4ic.config.ConfigProblem;
@@ -29,6 +30,8 @@
 import java.util.Map;
 
 public class SimpleDataPart implements DataPart {
+    private static final Logger LOG = Logger.getInstance(SimpleDataPart.class);
+
     static final String TAG_NAME = "simple-data-part";
     static final ConfigPartFactory<SimpleDataPart> FACTORY = new Factory();
     private static final String PROPERTY_TAG_NAME = "prop";
@@ -341,13 +344,17 @@ public void setLoginSsoFile(@Nullable File file) {
     @Override
     public Element marshal() {
         Element ret = new Element(TAG_NAME);
+        // FIXME debug
+        LOG.info("Marshalling " + properties);
         for (Map.Entry<String, String> entry : properties.entrySet()) {
             Element prop = new Element(PROPERTY_TAG_NAME);
             prop.setAttribute(KEY_ATTRIBUTE_NAME, entry.getKey());
             if (entry.getValue() != null) {
                 prop.setAttribute(VALUE_ATTRIBUTE_NAME, entry.getValue());
             }
+            ret.addContent(prop);
         }
+        LOG.info("Final marshal: " + ret);
         return ret;
     }
 
@@ -404,8 +411,11 @@ private boolean isTrimmedKeySet(@NotNull String key) {
     private void setTrimmed(@NotNull String key, @Nullable String value) {
         value = trimmedValue(value);
         if (value == null) {
+            // FIXME debug
+            LOG.info("Removing key " + key);
             properties.remove(key);
         } else {
+            LOG.info("Setting key " + key + " = [" + value + "]");
             properties.put(key, value);
         }
     }

diff --git a/plugin/src/net/groboclown/idea/p4ic/ui/config/props/PropertyConfigPanel.java b/plugin/src/net/groboclown/idea/p4ic/ui/config/props/PropertyConfigPanel.java
@@ -14,13 +14,16 @@
 
 package net.groboclown.idea.p4ic.ui.config.props;
 
+import com.intellij.openapi.fileChooser.FileChooserDescriptor;
+import com.intellij.openapi.fileChooser.FileChooserDescriptorFactory;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.ui.TextFieldWithBrowseButton;
+import com.intellij.openapi.util.io.FileUtil;
 import com.jgoodies.forms.layout.CellConstraints;
 import com.jgoodies.forms.layout.FormLayout;
 import net.groboclown.idea.p4ic.P4Bundle;
-import net.groboclown.idea.p4ic.config.P4ProjectConfig;
 import net.groboclown.idea.p4ic.config.part.SimpleDataPart;
+import net.groboclown.idea.p4ic.util.EqualUtil;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -58,9 +61,23 @@ public class PropertyConfigPanel
 
         authTicketFileFieldLabel.setLabelFor(authTicketFileField);
         authTicketFileField.setText(nullEmptyFile(project, part.getAuthTicketFile()));
+        authTicketFileField.setButtonEnabled(true);
+        authTicketFileField.addBrowseFolderListener(
+                P4Bundle.getString("configuration.properties.authticket.chooser.title"),
+                P4Bundle.getString("configuration.properties.authticket.chooser.desc"),
+                project,
+                FileChooserDescriptorFactory.createSingleLocalFileDescriptor()
+        );
 
         trustTicketFileFieldLabel.setLabelFor(trustTicketFileField);
         trustTicketFileField.setText(nullEmptyFile(project, part.getTrustTicketFile()));
+        trustTicketFileField.setButtonEnabled(true);
+        trustTicketFileField.addBrowseFolderListener(
+                P4Bundle.getString("configuration.properties.trustticket.chooser.title"),
+                P4Bundle.getString("configuration.properties.trustticket.chooser.desc"),
+                project,
+                FileChooserDescriptorFactory.createSingleLocalFileDescriptor()
+        );
 
         hostnameField.setText(nullEmptyTrim(part.getClientHostname()));
 
@@ -70,6 +87,13 @@ public class PropertyConfigPanel
 
         loginSsoFieldLabel.setLabelFor(loginSsoField);
         loginSsoField.setText(nullEmptyFile(project, part.getLoginSso()));
+        loginSsoField.setButtonEnabled(true);
+        loginSsoField.addBrowseFolderListener(
+                P4Bundle.getString("configuration.properties.loginsso.chooser.title"),
+                P4Bundle.getString("configuration.properties.loginsso.chooser.desc"),
+                project,
+                FileChooserDescriptorFactory.createSingleFileOrExecutableAppDescriptor()
+        );
     }
 
     @Nls
@@ -105,7 +129,28 @@ SimpleDataPart copyPart() {
 
     @Override
     public boolean isModified(@NotNull SimpleDataPart originalPart) {
-        return !originalPart.equals(getConfigPart());
+        return !isNotEqual(charsetField, originalPart.getDefaultCharset())
+            && !isNotEqual(ignoreFileNameField, originalPart.getIgnoreFileName())
+            && !isNotEqual(hostnameField, originalPart.getClientHostname())
+            && !isNotEqual(trustTicketFileField, originalPart.getTrustTicketFile())
+            && !isNotEqual(authTicketFileField, originalPart.getTrustTicketFile())
+            && !isNotEqual(userField, originalPart.getUsername())
+            && !isNotEqual(portField, originalPart.getRawServerName())
+            && !isNotEqual(loginSsoField, originalPart.getLoginSso());
+    }
+
+    private static boolean isNotEqual(@NotNull JTextField field, @Nullable String value) {
+        return EqualUtil.isEqual(field.getText(), value);
+    }
+
+    private static boolean isNotEqual(@NotNull TextFieldWithBrowseButton field, @Nullable File file) {
+        final File f;
+        if (field.getText().isEmpty()) {
+            f = null;
+        } else {
+            f = new File(field.getText());
+        }
+        return FileUtil.filesEqual(f, file);
     }
 
     @NotNull