GNOME Bugzilla – Bug 309187
Preselected highlighting mode in preferences
Last modified: 2005-08-01 17:43:14 UTC
The highlighting mode of the current file should be selected in the highlighting mode in the Syntax highlighting tab in preferences. Currently the highligh mode faults empty even if the editor "knows" the highlighling mode of the current file. Other information:
Correction: the highlighting mode always defaults "Ada".
Note that the preferences dialog is not modal, so the concept of "current file" is not very well defined in this case. You mean the file that was active when the preferences dialog has been opened. Right?
That solution sounds good enough. :-)
Created attachment 50010 [details] [review] gedit-preselect-highlighting.patch Made a quick patch for this (looks like it's indented wrong (tabs/spaces), sorry). As I have very little experience doing GTK+ programming, I'd like to get some feedback whether or not this is the sane way to do this. Works fine though. gedit-preferences-dialog.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+)
Created attachment 50012 [details] [review] Correctly formatted patch Fixed the formatting
Comment on attachment 50012 [details] [review] Correctly formatted patch The patch looks really good to me. A few cosmetic comments: >+ current_document = gedit_get_active_document(); Add a space before left bracket. >+ gtk_option_menu_set_history (GTK_OPTION_MENU (dlg->hl_mode_optionmenu), >+ selected_language); Align selected_language just below GTK_OPTION_MENU Please, re-attach the patch with a ChangeLog entry and I will commit it for you. When you create a patch, please use the -p option too (for diff). Thanks for you help.
another minor cosmetic comment: braces on their own line. if (foo) { lalal; }
there is another issue actually: gedit_get_active_document() may return NULL, so you need to handle that case.
> ------- Additional Comment #8 From paolo borelli 2005-08-01 10:33 UTC ------- > there is another issue actually: gedit_get_active_document() may return NULL, > so you need to handle that case. This shouldn't be a problem, gedit_document_get_language (GeditDocument *doc) contains a "g_return_val_if_fail (GEDIT_IS_DOCUMENT (doc), NULL);" line. Current_document_language would then be NULL and never match the in-progress language. Selected_language defaults to 0, so it would simply select the first item. Or am I failing to see something crucial here?
Created attachment 50059 [details] [review] Cleaned patch New patch, not sure if the ChangeLog entry is OK though.
g_return_val_if_fail is substantially an assertion of something that should never happen, when the condition is hit a critical warning is printed on screen. Beside these assertion can be disabled in distro builds, so you should never rely on them.
Created attachment 50064 [details] [review] Catch NULL from gedit_get_active_document () OK, added the assertion in real code, new patch attached: + current_document = gedit_get_active_document (); + if (current_document != NULL) + { + current_document_language = gedit_document_get_language ( + current_document); + } + else + { + current_document_language = NULL; + }
Comment on attachment 50064 [details] [review] Catch NULL from gedit_get_active_document () Thank you very much for the patch. It looks good to me. I will test and commit it ASAP.
Committed with some cosmetic changes. 2005-08-01 Ruben Vermeersch <ruben@Lambda1.be> Fixed bug #309187 (Preselected highlighting mode in preferences) * dialogs/gedit-preferences-dialog.c setup_syntax_highlighting_page): Automatically select the language of the active document.