Make "modifies current document" its own property. Add some tool tips.

src/plugins/coreplugin/dialogs/externaltoolconfig.cpp

@@ -73,7 +73,8 @@ QString ExternalToolConfig::searchKeywords() const
             << ui->workingDirectoryLabel->text()
             << ui->outputLabel->text()
             << ui->errorOutputLabel->text()
-            << ui->inputCheckbox->text();
+            << ui->modifiesDocumentCheckbox->text()
+            << ui->inputLabel->text();
     return keywords;
 }
 
@@ -128,7 +129,7 @@ void ExternalToolConfig::showInfoForItem(QTreeWidgetItem *item)
     ui->workingDirectory->setPath(tool->workingDirectory());
     ui->outputBehavior->setCurrentIndex((int)tool->outputHandling());
     ui->errorOutputBehavior->setCurrentIndex((int)tool->errorHandling());
-    ui->inputCheckbox->setChecked(!tool->input().isEmpty());
+    ui->modifiesDocumentCheckbox->setChecked(tool->modifiesCurrentDocument());
     ui->inputText->setPlainText(tool->input());
     ui->description->setCursorPosition(0);
     ui->arguments->setCursorPosition(0);

src/plugins/coreplugin/dialogs/externaltoolconfig.ui

@@ -87,6 +87,14 @@
       </item>
       <item row="4" column="0">
        <widget class="QLabel" name="outputLabel">
+        <property name="toolTip">
+         <string>&lt;!DOCTYPE HTML PUBLIC &quot;-//W3C//DTD HTML 4.0//EN&quot; &quot;http://www.w3.org/TR/REC-html40/strict.dtd&quot;&gt;
+&lt;html&gt;&lt;head&gt;&lt;meta name=&quot;qrichtext&quot; content=&quot;1&quot; /&gt;&lt;style type=&quot;text/css&quot;&gt;
+p, li { white-space: pre-wrap; }
+&lt;/style&gt;&lt;/head&gt;&lt;body style=&quot; font-family:'Lucida Grande'; font-size:13pt; font-weight:400; font-style:normal;&quot;&gt;
+&lt;p style=&quot; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;&quot;&gt;What to do with the executable's standard output.
+&lt;ul&gt;&lt;li&gt;Ignore: Do nothing with it&lt;/li&gt;&lt;li&gt;Show in pane: Show it in the general output pane&lt;/li&gt;&lt;li&gt;Replace selection: Replace the current selection in the current document with it&lt;/li&gt;&lt;/ul&gt;&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>
+        </property>
         <property name="text">
          <string>Output:</string>
         </property>
@@ -109,15 +117,21 @@
           <string>Replace selection</string>
          </property>
         </item>
-        <item>
-         <property name="text">
-          <string>Reload document</string>
-         </property>
-        </item>
        </widget>
       </item>
       <item row="5" column="0">
        <widget class="QLabel" name="errorOutputLabel">
+        <property name="toolTip">
+         <string>&lt;!DOCTYPE HTML PUBLIC &quot;-//W3C//DTD HTML 4.0//EN&quot; &quot;http://www.w3.org/TR/REC-html40/strict.dtd&quot;&gt;
+&lt;html&gt;&lt;head&gt;&lt;meta name=&quot;qrichtext&quot; content=&quot;1&quot; /&gt;&lt;style type=&quot;text/css&quot;&gt;
+p, li { white-space: pre-wrap; }
+&lt;/style&gt;&lt;/head&gt;&lt;body style=&quot; font-family:'Lucida Grande'; font-size:13pt; font-weight:400; font-style:normal;&quot;&gt;
+&lt;p style=&quot; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;&quot;&gt;What to do with the executable's standard error output.&lt;/p&gt;
+&lt;ul&gt;&lt;li&gt;Ignore: Do nothing with it&lt;/li&gt;
+&lt;li&gt;Show in pane: Show it in the general output pane&lt;/li&gt;
+&lt;li&gt;Replace selection: Replace the current selection in the current document with it&lt;/li&gt;
+&lt;/ul&gt;&lt;/body&gt;&lt;/html&gt;</string>
+        </property>
         <property name="text">
          <string>Error output:</string>
         </property>
@@ -140,27 +154,35 @@
           <string>Replace selection</string>
          </property>
         </item>
-        <item>
-         <property name="text">
-          <string>Reload document</string>
-         </property>
-        </item>
        </widget>
       </item>
-      <item row="6" column="0">
+      <item row="7" column="0">
-       <widget class="QCheckBox" name="inputCheckbox">
+       <widget class="QLabel" name="inputLabel">
+        <property name="toolTip">
+         <string>Text to pass to the executable via standard input. Leave empty if the executable should not receive any input.</string>
+        </property>
         <property name="text">
          <string>Input:</string>
         </property>
        </widget>
       </item>
-      <item row="6" column="1">
+      <item row="7" column="1">
        <widget class="QPlainTextEdit" name="inputText">
         <property name="lineWrapMode">
          <enum>QPlainTextEdit::NoWrap</enum>
         </property>
        </widget>
       </item>
+      <item row="6" column="1">
+       <widget class="QCheckBox" name="modifiesDocumentCheckbox">
+        <property name="toolTip">
+         <string>If the tool modifies the current document, set this flag to ensure that the document is saved before running the tool and is reloaded after the tool finished.</string>
+        </property>
+        <property name="text">
+         <string>Modifies current document</string>
+        </property>
+       </widget>
+      </item>
      </layout>
     </widget>
    </item>

src/plugins/coreplugin/externaltool.cpp

@@ -70,8 +70,12 @@ namespace {
     const char * const kError = "error";
     const char * const kOutputShowInPane = "showinpane";
     const char * const kOutputReplaceSelection = "replaceselection";
-    const char * const kOutputReloadDocument = "reloaddocument";
     const char * const kOutputIgnore = "ignore";
+    const char * const kModifiesDocument = "modifiesdocument";
+    const char * const kYes = "yes";
+    const char * const kNo = "no";
+    const char * const kTrue= "true";
+    const char * const kFalse = "false";
 }
 
 // #pragma mark -- ExternalTool
@@ -80,6 +84,7 @@ ExternalTool::ExternalTool() :
     m_order(-1),
     m_outputHandling(ShowInPane),
     m_errorHandling(ShowInPane),
+    m_modifiesCurrentDocument(false),
     m_isDisplayNameChanged(false)
 {
 }
@@ -96,6 +101,7 @@ ExternalTool::ExternalTool(const ExternalTool *other)
       m_workingDirectory(other->m_workingDirectory),
       m_outputHandling(other->m_outputHandling),
       m_errorHandling(other->m_errorHandling),
+      m_modifiesCurrentDocument(other->m_modifiesCurrentDocument),
       m_isDisplayNameChanged(other->m_isDisplayNameChanged)
 {
 }
@@ -167,6 +173,11 @@ ExternalTool::OutputHandling ExternalTool::errorHandling() const
     return m_errorHandling;
 }
 
+bool ExternalTool::modifiesCurrentDocument() const
+{
+    return m_modifiesCurrentDocument;
+}
+
 static QStringList splitLocale(const QString &locale)
 {
     QString value = locale;
@@ -217,12 +228,10 @@ static bool parseOutputAttribute(const QString &attribute, QXmlStreamReader *rea
         *value = ExternalTool::ShowInPane;
     } else if (output == QLatin1String(kOutputReplaceSelection)) {
         *value = ExternalTool::ReplaceSelection;
-    } else if (output == QLatin1String(kOutputReloadDocument)) {
-        *value = ExternalTool::ReloadDocument;
     } else if (output == QLatin1String(kOutputIgnore)) {
         *value = ExternalTool::Ignore;
     } else {
-        reader->raiseError(QLatin1String("Allowed values for output attribute are 'showinpane','replaceselection','reloaddocument'"));
+        reader->raiseError(QLatin1String("Allowed values for output attribute are 'showinpane','replaceselection','ignore'"));
         return false;
     }
     return true;
@@ -267,6 +276,17 @@ ExternalTool * ExternalTool::createFromXml(const QByteArray &xml, QString *error
                 if (!parseOutputAttribute(QLatin1String(kError), &reader, &tool->m_errorHandling))
                     break;
             }
+            if (reader.attributes().hasAttribute(QLatin1String(kModifiesDocument))) {
+                const QString &value = reader.attributes().value(QLatin1String(kModifiesDocument)).toString();
+                if (value == QLatin1String(kYes) || value == QLatin1String(kTrue)) {
+                    tool->m_modifiesCurrentDocument = true;
+                } else if (value == QLatin1String(kNo) || value == QLatin1String(kFalse)) {
+                    tool->m_modifiesCurrentDocument = false;
+                } else {
+                    reader.raiseError(QLatin1String("Allowed values for modifiesdocument attribute are 'yes','true','no','false'"));
+                    break;
+                }
+            }
             while (reader.readNextStartElement()) {
                 if (reader.name() == QLatin1String(kPath)) {
                     tool->m_executables.append(reader.readElementText());
@@ -319,6 +339,7 @@ bool ExternalTool::operator==(const ExternalTool &other)
             && m_input == other.m_input
             && m_workingDirectory == other.m_workingDirectory
             && m_outputHandling == other.m_outputHandling
+            && m_modifiesCurrentDocument == other.m_modifiesCurrentDocument
             && m_errorHandling == other.m_errorHandling;
 }
 
@@ -376,8 +397,7 @@ void ExternalToolRunner::run()
         deleteLater();
         return;
     }
-    if (m_tool->outputHandling() == ExternalTool::ReloadDocument
+    if (m_tool->modifiesCurrentDocument()) {
-               || m_tool->errorHandling() == ExternalTool::ReloadDocument) {
         if (IEditor *editor = EditorManager::instance()->currentEditor()) {
             m_expectedFileName = editor->file()->fileName();
             bool cancelled = false;
@@ -421,8 +441,8 @@ void ExternalToolRunner::finished(int exitCode, QProcess::ExitStatus status)
         if (m_tool->outputHandling() == ExternalTool::ReplaceSelection
                 || m_tool->errorHandling() == ExternalTool::ReplaceSelection) {
             emit ExternalToolManager::instance()->replaceSelectionRequested(m_processOutput);
-        } else if (m_tool->outputHandling() == ExternalTool::ReloadDocument
+        }
-                || m_tool->errorHandling() == ExternalTool::ReloadDocument) {
+        if (m_tool->modifiesCurrentDocument()) {
             FileManager::instance()->unexpectFileChange(m_expectedFileName);
         }
     }
@@ -433,8 +453,7 @@ void ExternalToolRunner::finished(int exitCode, QProcess::ExitStatus status)
 
 void ExternalToolRunner::error(QProcess::ProcessError error)
 {
-    if (m_tool->outputHandling() == ExternalTool::ReloadDocument
+    if (m_tool->modifiesCurrentDocument()) {
-            || m_tool->errorHandling() == ExternalTool::ReloadDocument) {
         FileManager::instance()->unexpectFileChange(m_expectedFileName);
     }
     // TODO inform about errors

src/plugins/coreplugin/externaltool.h

@@ -54,8 +54,7 @@ public:
     enum OutputHandling {
         Ignore,
         ShowInPane,
-        ReplaceSelection,
+        ReplaceSelection
-        ReloadDocument
     };
 
     ExternalTool();
@@ -70,6 +69,7 @@ public:
     int order() const;
     OutputHandling outputHandling() const;
     OutputHandling errorHandling() const;
+    bool modifiesCurrentDocument() const;
 
     QStringList executables() const;
     QString arguments() const;
@@ -95,6 +95,8 @@ private:
     QString m_workingDirectory;
     OutputHandling m_outputHandling;
     OutputHandling m_errorHandling;
+    bool m_modifiesCurrentDocument;
+
     bool m_isDisplayNameChanged;
 };
 

src/share/qtcreator/externaltools/vi.xml

@@ -36,7 +36,7 @@
     <displayname xml:lang="de">In vi öffnen</displayname>
     <category>Text</category>
     <category xml:lang="de">Text</category>
-    <executable output="reloaddocument">
+    <executable modifiesdocument="yes">
         <path>xterm</path>
         <arguments>-geom %{CurrentDocument:ColumnCount}x%{CurrentDocument:RowCount}+%{CurrentDocument:XPos}+%{CurrentDocument:YPos} -e vi "%{CurrentDocument:FilePath}" +%{CurrentDocument:Row} +"normal %{CurrentDocument:Column}|"</arguments>
         <workingdirectory>%{CurrentDocument:Path}</workingdirectory>

src/share/qtcreator/externaltools/vi_mac.xml

@@ -36,7 +36,7 @@
     <displayname xml:lang="de">In vi öffnen</displayname>
     <category>Text</category>
     <category xml:lang="de">Text</category>
-    <executable output="reloaddocument">
+    <executable modifiesdocument="yes">
         <path>osascript</path>
         <input>
         --Terminal opens a window by default when it is not running, so check

tests/auto/externaltool/tst_externaltooltest.cpp

@@ -49,7 +49,7 @@ static const char * const TEST_XML3 =
 "    <displayname xml:lang=\"de\">In vi öffnen</displayname>"
 "    <category>Text</category>"
 "    <category xml:lang=\"de\">Text</category>"
-"    <executable output=\"reloaddocument\">"
+"    <executable modifiesdocument=\"yes\">"
 "        <path>xterm</path>"
 "        <arguments>-geom %{EditorCharWidth}x%{EditorCharHeight}+%{EditorXPos}+%{EditorYPos} -e vi %{CurrentFilePath} +%{EditorLine} +\"normal %{EditorColumn}|\"</arguments>"
 "        <workingdirectory>%{CurrentPath}</workingdirectory>"
@@ -143,7 +143,8 @@ void ExternaltoolTest::testRead3()
     QVERIFY(tool->arguments().startsWith(QLatin1String("-geom %{")));
     QCOMPARE(tool->input(), QString());
     QCOMPARE(tool->workingDirectory(), QString::fromLatin1("%{CurrentPath}"));
-    QCOMPARE(tool->outputHandling(), ExternalTool::ReloadDocument);
+    QCOMPARE(tool->outputHandling(), ExternalTool::ShowInPane);
+    QCOMPARE(tool->modifiesCurrentDocument(), true);
     QCOMPARE(tool->errorHandling(), ExternalTool::ShowInPane);
     delete tool;
 }