STORM-1018 FIXED Improved error messaging for the External Editor feature.

Let the user know what's wrong with external editor.

Added meaningful messages for the following errors:
* Editor not specified.
* Error parsing command line.
* Specified binary not found.
* Editor failed to run.

All the messages are translatable.

indra/newview/llexternaleditor.cpp

@@ -27,6 +27,7 @@
 #include "llviewerprecompiledheaders.h"
 #include "llexternaleditor.h"
 
+#include "lltrans.h"
 #include "llui.h"
 
 // static
@@ -35,13 +36,13 @@ const std::string LLExternalEditor::sFilenameMarker = "%s";
 // static
 const std::string LLExternalEditor::sSetting = "ExternalEditor";
 
-bool LLExternalEditor::setCommand(const std::string& env_var, const std::string& override)
+LLExternalEditor::EErrorCode LLExternalEditor::setCommand(const std::string& env_var, const std::string& override)
 {
 	std::string cmd = findCommand(env_var, override);
 	if (cmd.empty())
 	{
-		llwarns << "Empty editor command" << llendl;
-		return false;
+		llwarns << "Editor command is empty or not set" << llendl;
+		return EC_NOT_SPECIFIED;
 	}
 
 	// Add the filename marker if missing.
@@ -55,7 +56,7 @@ bool LLExternalEditor::setCommand(const std::string& env_var, const std::string&
 	if (tokenize(tokens, cmd) < 2) // 2 = bin + at least one arg (%s)
 	{
 		llwarns << "Error parsing editor command" << llendl;
-		return false;
+		return EC_PARSE_ERROR;
 	}
 
 	// Check executable for existence.
@@ -63,7 +64,7 @@ bool LLExternalEditor::setCommand(const std::string& env_var, const std::string&
 	if (!LLFile::isfile(bin_path))
 	{
 		llwarns << "Editor binary [" << bin_path << "] not found" << llendl;
-		return false;
+		return EC_BINARY_NOT_FOUND;
 	}
 
 	// Save command.
@@ -76,16 +77,16 @@ bool LLExternalEditor::setCommand(const std::string& env_var, const std::string&
 	}
 	llinfos << "Setting command [" << bin_path << " " << mArgs << "]" << llendl;
 
-	return true;
+	return EC_SUCCESS;
 }
 
-bool LLExternalEditor::run(const std::string& file_path)
+LLExternalEditor::EErrorCode LLExternalEditor::run(const std::string& file_path)
 {
 	std::string args = mArgs;
 	if (mProcess.getExecutable().empty() || args.empty())
 	{
 		llwarns << "Editor command not set" << llendl;
-		return false;
+		return EC_NOT_SPECIFIED;
 	}
 
 	// Substitute the filename marker in the command with the actual passed file name.
@@ -111,7 +112,22 @@ bool LLExternalEditor::run(const std::string& file_path)
 		mProcess.orphan();
 	}
 
-	return result == 0;
+	return result == 0 ? EC_SUCCESS : EC_FAILED_TO_RUN;
+}
+
+// static
+std::string LLExternalEditor::getErrorMessage(EErrorCode code)
+{
+	switch (code)
+	{
+	case EC_SUCCESS: 			return LLTrans::getString("ok");
+	case EC_NOT_SPECIFIED: 		return LLTrans::getString("ExternalEditorNotSet");
+	case EC_PARSE_ERROR:		return LLTrans::getString("ExternalEditorCommandParseError");
+	case EC_BINARY_NOT_FOUND:	return LLTrans::getString("ExternalEditorNotFound");
+	case EC_FAILED_TO_RUN:		return LLTrans::getString("ExternalEditorFailedToRun");
+	}
+
+	return LLTrans::getString("Unknown");
 }
 
 // static

indra/newview/llexternaleditor.h

@@ -42,6 +42,14 @@ class LLExternalEditor
 
 public:
 
+	typedef enum e_error_code {
+		EC_SUCCESS,				/// No error.
+		EC_NOT_SPECIFIED,		/// Editor path not specified.
+		EC_PARSE_ERROR,			/// Editor command parsing error.
+		EC_BINARY_NOT_FOUND,	/// Could find the editor binary (missing or not quoted).
+		EC_FAILED_TO_RUN,		/// Could not execute the editor binary.
+	} EErrorCode;
+
 	/**
 	 * Set editor command.
 	 *
@@ -51,19 +59,25 @@ class LLExternalEditor
 	 * First tries the override, then a predefined setting (sSetting),
 	 * then the environment variable.
 	 *
-	 * @return Command if found, empty string otherwise.
+	 * @return EC_SUCCESS if command is valid and refers to an existing executable,
+	 *         EC_NOT_SPECIFIED or EC_FAILED_TO_RUNan on error.
 	 *
 	 * @see sSetting
 	 */
-	bool setCommand(const std::string& env_var, const std::string& override = LLStringUtil::null);
+	EErrorCode setCommand(const std::string& env_var, const std::string& override = LLStringUtil::null);
 
 	/**
 	 * Run the editor with the given file.
 	 *
 	 * @param file_path File to edit.
-	 * @return true on success, false on error.
+	 * @return EC_SUCCESS on success, error code on error.
+	 */
+	EErrorCode run(const std::string& file_path);
+
+	/**
+	 * Get a meaningful error message for the given status code.
 	 */
-	bool run(const std::string& file_path);
+	static std::string getErrorMessage(EErrorCode code);
 
 private:
 

indra/newview/llfloateruipreview.cpp

@@ -1037,18 +1037,29 @@ void LLFloaterUIPreview::onClickEditFloater()
 			cmd_override = bin + " " + args;
 		}
 	}
-	if (!mExternalEditor.setCommand("LL_XUI_EDITOR", cmd_override))
+
+	LLExternalEditor::EErrorCode status = mExternalEditor.setCommand("LL_XUI_EDITOR", cmd_override);
+	if (status != LLExternalEditor::EC_SUCCESS)
+	{
+		std::string warning;
+
+		if (status == LLExternalEditor::EC_NOT_SPECIFIED) // Use custom message for this error.
+		{
+			warning = getString("ExternalEditorNotSet");
+		}
+		else
 		{
-		std::string warning = "Select an editor by setting the environment variable LL_XUI_EDITOR "
-			"or the ExternalEditor setting or specifying its path in the \"Editor Path\" field.";
+			warning = LLExternalEditor::getErrorMessage(status);
+		}
+
 		popupAndPrintWarning(warning);
 		return;
 	}
 
 	// Run the editor.
-	if (!mExternalEditor.run(file_path))
+	if (mExternalEditor.run(file_path) != LLExternalEditor::EC_SUCCESS)
 	{
-		popupAndPrintWarning("Failed to run editor");
+		popupAndPrintWarning(LLExternalEditor::getErrorMessage(status));
 		return;
 	}
 }

indra/newview/llpreviewscript.cpp

@@ -956,16 +956,31 @@ void LLScriptEdCore::openInExternalEditor()
 	// Open it in external editor.
 	{
 		LLExternalEditor ed;
+		LLExternalEditor::EErrorCode status;
+		std::string msg;
 
-		if (!ed.setCommand("LL_SCRIPT_EDITOR"))
+		status = ed.setCommand("LL_SCRIPT_EDITOR");
+		if (status != LLExternalEditor::EC_SUCCESS)
 		{
-			std::string msg = "Select an editor by setting the environment variable LL_SCRIPT_EDITOR "
-				"or the ExternalEditor setting"; // *TODO: localize
+			if (status == LLExternalEditor::EC_NOT_SPECIFIED) // Use custom message for this error.
+			{
+				msg = getString("external_editor_not_set");
+			}
+			else
+			{
+				msg = LLExternalEditor::getErrorMessage(status);
+			}
+
 			LLNotificationsUtil::add("GenericAlert", LLSD().with("MESSAGE", msg));
 			return;
 		}
 
-		ed.run(filename);
+		status = ed.run(filename);
+		if (status != LLExternalEditor::EC_SUCCESS)
+		{
+			msg = LLExternalEditor::getErrorMessage(status);
+			LLNotificationsUtil::add("GenericAlert", LLSD().with("MESSAGE", msg));
+		}
 	}
 }
 

indra/newview/skins/default/xui/en/floater_ui_preview.xml

@@ -12,6 +12,10 @@
  title="XUI PREVIEW TOOL"
  translate="false"
  width="750">
+    <string name="ExternalEditorNotSet">
+Select an editor by setting the environment variable LL_XUI_EDITOR
+or the ExternalEditor setting
+or specifying its path in the "Editor Path" field.</string>
     <panel
      bottom="640"
      follows="left|top|right|bottom"

indra/newview/skins/default/xui/en/panel_script_ed.xml

@@ -28,6 +28,10 @@
      name="Title">
         Script: [NAME]
     </panel.string>
+    <panel.string
+     name="external_editor_not_set">
+        Select an editor by setting the environment variable LL_SCRIPT_EDITOR or the ExternalEditor setting.
+    </panel.string>
     <menu_bar
      bg_visible="false"
      follows="left|top"

indra/newview/skins/default/xui/en/strings.xml

@@ -3311,6 +3311,14 @@ Abuse Report</string>
 
   <string name="EmptyOutfitText">There are no items in this outfit</string>
  
+ <!-- External editor status codes -->
+ <string name="ExternalEditorNotSet">Select an editor using the ExternalEditor setting.</string>
+ <string name="ExternalEditorNotFound">Cannot find the external editor you specified.
+Try enclosing path to the editor with double quotes.
+(e.g. "/path to my/editor" "%s")</string>
+ <string name="ExternalEditorCommandParseError">Error parsing the external editor command.</string>
+ <string name="ExternalEditorFailedToRun">External editor failed to run.</string>
+
   <!-- Key names begin -->
   <string name="Esc">Esc</string>
   <string name="Space">Space</string>