[opensource-dev] Review Request: STORM-1018 Improve error messaging for External Editor feature
Merov Linden
merov at lindenlab.com
Mon Mar 7 22:31:53 PST 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/179/#review432
-----------------------------------------------------------
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.
- Merov
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/20110308/ca7f2e8b/attachment-0001.htm
More information about the opensource-dev
mailing list