> 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 > >
_______________________________________________ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
