[opensource-dev] Review Request: STORM-1018 Improve error messaging for External Editor feature

Vadim ProductEngine vsavchuk at productengine.com
Wed Mar 9 03:57:33 PST 2011



> On March 7, 2011, 10:31 p.m., Merov Linden wrote:
> > I like the intent. In the code though, I don't really understand why the EC_NOT_SPECIFIED case is treated differently than other error messages. I fail to understand why this needs a "custom" message (in the panel xml instead of strings.xml) and why having it in the panel xml has any advantage.

It's because the XUI preview tool and script editing provide different ways to specify external editor.

Script editing: ExternalEditor setting, which can be overriden with LL_SCRIPT_EDITOR environment variable.

XUI preview tool: ExternalEditor settings, which can be overriden with LL_XUI_EDITOR that, in turn, is overridable with the Editor Path input field of the floater.

Thus we need different warning messages for the two cases.


- Vadim


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/179/#review432
-----------------------------------------------------------


On March 7, 2011, 9:16 a.m., Vadim ProductEngine wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/179/
> -----------------------------------------------------------
> 
> (Updated March 7, 2011, 9:16 a.m.)
> 
> 
> Review request for Viewer and Seth ProductEngine.
> 
> 
> Summary
> -------
> 
> 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.
> 
> 
> This addresses bug STORM-1018.
>     http://jira.secondlife.com/browse/STORM-1018
> 
> 
> Diffs
> -----
> 
>   indra/newview/llexternaleditor.h ef2df52563bb 
>   indra/newview/llexternaleditor.cpp ef2df52563bb 
>   indra/newview/llfloateruipreview.cpp ef2df52563bb 
>   indra/newview/llpreviewscript.cpp ef2df52563bb 
>   indra/newview/skins/default/xui/en/floater_ui_preview.xml ef2df52563bb 
>   indra/newview/skins/default/xui/en/panel_script_ed.xml ef2df52563bb 
>   indra/newview/skins/default/xui/en/strings.xml ef2df52563bb 
> 
> Diff: http://codereview.secondlife.com/r/179/diff
> 
> 
> Testing
> -------
> 
> Test cases:
> 1. Use a path containing spaces without enclosing it with double quotes (/path to/editor).
>    Expected: the "not found" message.
> 2. Specify empty path ().
>    Expected: the "not found" message.
> 3. Try using an odd number of double quotes ("/path to/editor "%s").
>    Expected: the "parse error" message.
> 4. Specifying a nonexistent editor (/non/existent/editor).
>    Expected: the "not found" message.
> 5. Specify a valid editor path (/usr/bin/editor).
>    Expected: the editor is executed.
> 
> The command can be specified with the ExternalEditor debug setting or an environment variable: LL_SCRIPT_EDITOR for script editor and LL_XUI_EDITOR for the XUI preview tool. In the latter case you can also override the command via the "Editor Path" floater input field.
> 
> 
> Thanks,
> 
> Vadim
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110309/7d09b765/attachment.htm 


More information about the opensource-dev mailing list