Do

Merge lp://qastaging/~psybers/do/loglevel into lp://qastaging/do

Proposed by Robert Dyer
Status: Rejected
Rejected by: Chris S.
Proposed branch: lp://qastaging/~psybers/do/loglevel
Merge into: lp://qastaging/do
Diff against target: None lines
To merge this branch: bzr merge lp://qastaging/~psybers/do/loglevel
Reviewer Review Type Date Requested Status
Chris S. Disapprove
Chris Halse Rogers Pending
Review via email: mp+7316@code.qastaging.launchpad.net
To post a comment you must log in.
Revision history for this message
Robert Dyer (psybers) wrote :

Removes --debug, replaces with a --log <LEVEL> option where <LEVEL> is any valid value from the LogLevel enum.

Makes the static methods in CorePreferences non-static.

Revision history for this message
David Siegel (djsiegel-deactivatedaccount) wrote :

"opt" and "ret" are not good variable names. Please choose something meaningful, and don't use abbreviations.

lp://qastaging/~psybers/do/loglevel updated
1202. By Robert Dyer <rdyer@guyute>

changing variable names to be more descriptive

1203. By Robert Dyer <rdyer@guyute>

merge trunk

Revision history for this message
Robert Dyer (psybers) wrote :

> "opt" and "ret" are not good variable names. Please choose something
> meaningful, and don't use abbreviations.

Fixed in lp:~psybers/do/loglevel revision 1202
http://bazaar.launchpad.net/~psybers/do/loglevel/revision/1202

lp://qastaging/~psybers/do/loglevel updated
1204. By Robert Dyer <rdyer@narmada>

merge trunk

1205. By Robert Dyer <rdyer@narmada>

fix compile after trunk merge

Revision history for this message
Chris S. (cszikszoy) wrote :

Going with Mono.Options is probably a better idea. I'll get to it sometime soon.

review: Disapprove

Unmerged revisions

1205. By Robert Dyer <rdyer@narmada>

fix compile after trunk merge

1204. By Robert Dyer <rdyer@narmada>

merge trunk

1203. By Robert Dyer <rdyer@guyute>

merge trunk

1202. By Robert Dyer <rdyer@guyute>

changing variable names to be more descriptive

1201. By Robert Dyer <rdyer@yamuna>

removing the static from CorePreferences

1200. By Robert Dyer <rdyer@yamuna>

merge trunk

1199. By Robert Dyer <rdyer@yamuna>

Renaming the commandline argument to --arg
Using Enum.Parse instead of if/else

1198. By Robert Dyer <rdyer@yamuna>

Better fix for bug 380470 - 1 commandline option to set the loglevel

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Do/src/CorePreferences.cs'
2--- Do/src/CorePreferences.cs 2009-03-23 03:05:45 +0000
3+++ Do/src/CorePreferences.cs 2009-06-11 03:11:13 +0000
4@@ -42,8 +42,9 @@
5 const bool AlwaysShowResultsDefaultValue = false;
6 const string TextModeKeybindingDefaultValue = "period";
7 const string SummonKeybindingDefaultValue = "<Super>space";
8+ const string LogLevelDefaultValue = "Error";
9
10- const string DebugOption = "--debug";
11+ const string LogLevelOption = "--log";
12 const string LogToFileOption = "--log-to-file";
13
14
15@@ -59,18 +60,23 @@
16 Preferences.PreferencesChanged += PreferencesChanged;
17 }
18
19+ public LogLevel LogLevel {
20+ get {
21+ string opt = GetArgument (LogLevelOption, LogLevelDefaultValue);
22+
23+ try {
24+ LogLevel ret = (LogLevel) Enum.Parse (typeof (LogLevel), opt);
25+ if (Enum.IsDefined (typeof (LogLevel), ret))
26+ return ret;
27+ } catch (Exception) { }
28+ return LogLevel.Error;
29+ }
30+ }
31+
32 public bool WriteLogToFile {
33 get { return HasOption (LogToFileOption); }
34 }
35
36- public static bool PeekDebug {
37- get { return HasOption (DebugOption); }
38- }
39-
40- public bool Debug {
41- get { return CorePreferences.PeekDebug; }
42- }
43-
44 public string Theme {
45 get { return Preferences.Get (ThemeKey, ThemeDefaultValue); }
46 set { Preferences.Set (ThemeKey, value); }
47@@ -91,11 +97,21 @@
48 set { Preferences.Set (AlwaysShowResultsKey, value); }
49 }
50
51- static bool HasOption (string option)
52+ bool HasOption (string option)
53 {
54 return Env.GetCommandLineArgs ().Contains (option);
55 }
56
57+ string GetArgument (string option, string defaultValue)
58+ {
59+ string [] args = Env.GetCommandLineArgs ();
60+ int index = Array.LastIndexOf (args, option) + 1;
61+ if (index > 0 && index < args.Length)
62+ return args [index];
63+
64+ return defaultValue;
65+ }
66+
67
68 void PreferencesChanged (object sender, PreferencesChangedEventArgs e)
69 {
70
71=== modified file 'Do/src/Do.Core/PluginManager.cs'
72--- Do/src/Do.Core/PluginManager.cs 2009-06-05 19:45:45 +0000
73+++ Do/src/Do.Core/PluginManager.cs 2009-06-11 03:11:13 +0000
74@@ -60,7 +60,7 @@
75 /// Performs plugin system initialization. Should be called before this
76 /// class or any Mono.Addins class is used. The ordering is very delicate.
77 /// </summary>
78- public static void Initialize ()
79+ public static void Initialize (bool debug)
80 {
81 IEnumerable<string> savedPlugins = PluginsEnabledBeforeLoad ();
82
83@@ -71,7 +71,7 @@
84 // reenable them with the Id of the new version. It's a bit hackish but lluis
85 // said it's a reasonable approach until that bug is fixed
86 // https://bugzilla.novell.com/show_bug.cgi?id=490302
87- if (CorePreferences.PeekDebug)
88+ if (debug)
89 AddinManager.Registry.Rebuild (null);
90 else
91 AddinManager.Registry.Update (null);
92
93=== modified file 'Do/src/Do.cs'
94--- Do/src/Do.cs 2009-03-28 19:49:54 +0000
95+++ Do/src/Do.cs 2009-06-11 03:11:13 +0000
96@@ -45,25 +45,14 @@
97 Gtk.Application.Init ();
98 Gdk.Threads.Init ();
99
100- // We are conservative with the log at first.
101- Log.DisplayLevel = LogLevel.Error;
102- if (CorePreferences.PeekDebug)
103- Log.DisplayLevel = LogLevel.Debug;
104+ Preferences = new CorePreferences ();
105+ Log.DisplayLevel = Preferences.LogLevel;
106
107- PluginManager.Initialize ();
108+ PluginManager.Initialize (Preferences.LogLevel == LogLevel.Debug);
109 Services.System.EnsureSingleApplicationInstance ();
110
111- Preferences = new CorePreferences ();
112-
113 Keybindings = new CoreKeybindings ();
114
115- // Now we can set the preferred log level.
116- if (Preferences.QuietStart)
117- Log.DisplayLevel = LogLevel.Error;
118- // Check for debug again in case QuietStart is also set.
119- if (Preferences.Debug)
120- Log.DisplayLevel = LogLevel.Debug;
121-
122 try {
123 Util.SetProcessName ("gnome-do");
124 } catch (Exception e) {