GNOME Bugzilla – Bug 401888
The /apps/tomboy/menu_note_count GConf option should be made more accessible
Last modified: 2011-07-05 18:07:06 UTC
The /apps/tomboy/menu_note_count GConf option is quite a relevant option that should be made available to users through the Preferences dialog. I understand the philosophy of GNOME that users shouldn't be overwhelmed by too many options, but that option is a pretty common one and it'd make sense including it into the preferences dialog. Other information:
We're re-evaluate this for 0.8.0 (post GNOME 2.18).
Setting the default assignee and QA Contact to "tomboy-maint@gnome.bugs".
This was reported more than 2! years ago. I'd love to see a response that a) commits to an implementation soon (next release?) b) marks this as WONTFIX/OBSOLETE Someone?
2 month of silence again: Is this not a preference because we don't _want_ it to be exposed? In that case I'd close this as WONTFIX. If this is something that could be done/would be supported this would be a nice gnome-love candidate for example. Needs an update from the tomboy guys, imo. Accept or decline ;)
I'd say we should go ahead and add it to the preferences UI. Patches welcome.
I might look into it, but setting it to gnome-love for now. Should be a low hanging fruit for anyone interested. Thanks for the update.
Hi everyone, I've never submitted a patch to an open source project before, but I'd be interested in potentially cutting my teeth with this bug - seems like a pretty simple first step. Assuming folks would be agreeable to me helping out I'd like to hop right in. Sandy was very nice about pointing me in the right direction on IRC, but I may still be somewhat lost so I'll apologize in advance... I have a question before I go too far though: where in the preferences dialog should this option appear? As of a pull from git I made earlier today, there are four tabs in the preferences menu, and I'm not sure if this setting really falls under any of them. Any help or guidance would be appreciated. Thanks in advance.
Created attachment 179097 [details] [review] Proposed fix Please see the proposed fix attached. Introduces new "Misc" tab into Preferences dialog with only one setting so far in it. Currently the number of notes shown in Recent list is limited by hardcoded number 18 (limit from above, set as "max_size" in Tray.cs:442) and by the now configurable number (limit from below, default is 10, set as "MENU_NOTE_COUNT" in Preferences.cs:103-104). In view of that, caption for the option warns that 18 is the maximum number. I think in the future we may want to challenge this hardcoded value and/or turn it into option as well, but with current patch I just want to solve the exact problem the bug is about. Comments/suggestions as to the option wording as well as overall approach are appreciated.
Hi Alex, thanks for the patch. I'm not sure I want to add another tab to preferences, especially not one called "Misc". But I'm not a UX expert so I'd like to get some advice from some other folks on the best approach here. I'm not really sure where this option should go, to be honest. It might be the best thing is to create an optional add-in that adds an "Advanced" (or "Misc", or whatever) tab to the preferences and has options like this. If we go that way, it should be easy to adapt your work to that purpose.
So out of the options to force it into any of the currently well-labeled tabs, creating a new tab with just one control in it and making this an add-in, I feel an addin is the best alternative. Mostly because the first two bring too much user experience regression.
Created attachment 179598 [details] [review] Proposed fix, new Advanced preferences tab implemented via addin Sure, no problem, please see the attachment. Name for the tab changed to Advanced, "Misc" was just a shorter one, but I don't have any preferences for either one :)
Created attachment 179914 [details] [review] Small housekeeping for the previous patch (179598) - added Makefile, removed autogenerated by IDE *.cs file. Some housekeeping for the previous patch
So can we have this reviewed, should be rather quick one?
Will review next time I'm booted into Linux, may be a week or so. Don't like pushing stuff from Mac when I can't test autofoo properly. If another Tomboy dev wants to review/push instead, that's cool.
Comment on attachment 179914 [details] [review] Small housekeeping for the previous patch (179598) - added Makefile, removed autogenerated by IDE *.cs file. Looks good, but autotools are not generating the Makefile properly. You're missing an entry in configure.in
Alex, thanks for the patch, I'd like to get this in for the next release on Monday. When I enable the addin, I'm getting an error that the tab can't be added. Here is the exception: [DEBUG 09:25:32.650] Adding preference tab addin: AdvPrefTabAddin [WARN 09:25:32.655] Problems adding preferences tab addin: AdvPrefTabAddin [DEBUG 09:25:32.669] Cannot cast from source type to destination type.: at GConf.PropertyEditors.PropertyEditorEntry.ValueChanged (System.Object sender, GConf.NotifyEventArgs args) [0x00000] in <filename unknown>:0 at GConf.PropertyEditors.PropertyEditor.Setup () [0x00000] in <filename unknown>:0 at Tomboy.AdvPrefTab.AdvPrefTabAddin.GetPreferenceTabWidget (Tomboy.PreferencesDialog parent, System.String& tabLabel, Gtk.Widget& preferenceWidget) [0x000f6] in /home/aaborden/src/tomboy/Tomboy/Addins/AdvPrefTab/AdvPrefTabAddin.cs:65 at Tomboy.PreferencesDialog..ctor (Tomboy.AddinManager addin_manager) [0x00121] in /home/aaborden/src/tomboy/Tomboy/PreferencesDialog.cs:96
Oh, one comment, on the review I forgot to mention. Would you rename the addin to "AdvancedPreferencesAddin"? No need for the abbreviations.
Oops, somehow I missed emails related to this one in my inbox, I would've corrected this by release. Sorry for that, Aaron, and thanks for review. To make sure - you were testing on which OS, I see that's some UNIX? I've tested it on Win7x64 and (IIRC) openSUSE 11.3 x86_64, haven't seen such ones. I'll check it out for sure. Is that always reproducible or intermittent?
(In reply to comment #18) > Oops, somehow I missed emails related to this one in my inbox, I would've > corrected this by release. Sorry for that, Aaron, and thanks for review. No problem, it was very short notice. The addin will land in the next release which will be soon enough :) > To make sure - you were testing on which OS, I see that's some UNIX? I've > tested it on Win7x64 and (IIRC) openSUSE 11.3 x86_64, haven't seen such ones. > I'll check it out for sure. Is that always reproducible or intermittent? Yeah, it's Ubuntu 11.04 with Mono JIT compiler version 2.6.7 (Debian 2.6.7-5ubuntu3). Was always reproducible as far as I can tell.
Created attachment 190199 [details] [review] Proposed fix, incorporates all the feedback and fixes the exception seen on Linux (In reply to comment #16) > When I enable the addin, I'm getting an error that the tab can't be > added. Here is the exception: > > [DEBUG 09:25:32.650] Adding preference tab addin: AdvPrefTabAddin > [WARN 09:25:32.655] Problems adding preferences tab addin: AdvPrefTabAddin > [DEBUG 09:25:32.669] Cannot cast from source type to destination type.: This was caused by the fact I used GTK Entry and GConf PropertyEditorEntry, which expect to have a string as a value and use a (string) explicit typecast, while the config option is an integer. Apparently last time I haven't tested on Linux and on Windows everything works just fine (which is interesting, btw). To avoid getting too deep into modifying PropertyEditor class, I just too a different approach and (a) used SpinButton instead of Entry and (b) implemented an EventHandler that sets the option without PropertyEditor (similar to what Sync tab-related addin does). Attached is a new proposed fix, supersedes all previous patches, tested to cleanly apply to current tree, compile and work as intended on Win7x64 & openSUSE 11.3 x86_64. Comments are welcome.
Windows uses a different preferences backend with different property editor implementations, just fyi.
Review of attachment 190199 [details] [review]: ::: Tomboy/Addins/AdvancedPreferences/AdvancedPreferencesAddin.cs @@ +54,3 @@ + + // Menu Note Count option + label = new Gtk.Label (Catalog.GetString ("Number of notes to show in Recent list (max. 18)")); Sorry I missed this before. The string here is incorrect, the setting is actually the _minimum_ number of notes to show in the recent list.
Created attachment 190313 [details] [review] Proposed fix, new Advanced preferences tab implemented via addin Indeed, and I have noted this even by myself in several places in the source and in this discussion :) Here we go, a corrected patch.
In ignorance I ask; Why do we use Gconf? Since we must run on multiple platforms and Mono doesn't provide a single interface for ALL os's. Why don't we just store all settings in a custom XML file in the local configuration directory? This would save us from multiple OS issues It would allow users to sync config's between all PC's (possibly)
@Jared I was confused about this too, but sandy explained gconf is only used on Linux, the Preferences code acts as a proxy on different OS's. I'm not exactly sure what backend it uses for Windows and Mac. Is there a need for syncing tomboy settings? You might want to open a new bug to discuss a Preferences overhaul (there is already a bug to migrate to GSettings). @Alex Thanks for the patch, I'll take a look as soon as I get a chance.
We use GConf (and soon, GSettings) on Linux because that is the way users expect to consume GNOME application preferences. The most compelling use case is, perhaps, a system administrator deploying GNOME (with Tomboy) on hundreds of systems, and being able to use one tool to tweak the schemas for various apps' default preferences. On Mac and Windows we use the simple XML backend we wrote because enterprise deployments are not a realistic concern. We could use GSettings on those platforms too if we were content to depend on a more recent version of GLib/GTK+/whatever.
Comment on attachment 190313 [details] [review] Proposed fix, new Advanced preferences tab implemented via addin commit d3c796c6c0877b60c32d432e4e81cf5601a27e8c Thanks for the patch! I modified the string to expand the abbreviations to make it easier on translators.
Thanks! This will make it into the 1.7.2 release.