b4n commented on this pull request.

@@ -1,10 +0,0 @@

-@import "geany.css"; - -/* make close button on the editor's tabs smaller */ -#geany-close-tab-button { - -GtkWidget-focus-padding: 0; - -GtkWidget-focus-line-width: 0; - -GtkButton-default-border: 0; - -GtkButton-default-outside-border: 0; - -GtkButton-inner-border: 0; -}

IIUC this was conditional and not loaded on 3.20 because it was redundant, and would possibly lead to runtime deprecation warnings. Why drop it?

}

- g_object_unref(css); - g_free(css_file); -#else - /* see setup_gtk2_styles() in main.c */ + // if the user provided a geany.css file in their config dir, try and load that

Do we really need that? Isn't *~/.config/gtk-3.0/gtk.css* enough? AFAIK all our styles can easily be applied to Geany only, so I don't see why we'd need extra stuff?

@@ -494,6 +494,27 @@ An example of a simple ``.gtkrc-2.0``::

widget "GeanyPrefsDialog" style "geanyStyle"

+Customizing Geany's appearance using GTK+ 3 CSS +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +To override GTK+ CSS styles, you can use traditional mechanisms or you +can create a file named ``geany.css`` in the user configuration directory +(usually ``~/.config/geany``) which will be loaded after other CSS styles +are applied to allow overriding the default styles. + +Geany offers a number of CSS classes which can be overridden to taylor its

Those are CSS IDs (`#name`), not classes (`.name`)

@@ -83,6 +83,11 @@ enum

};

+static GdkColor color_error = {0, 65535, 0, 0}; +static GdkColor color_context = {0, 65535 / 2, 0, 0}; +static GdkColor color_message = {0, 0, 0, 0xD000};

different literals look odd, I'd rather use `0xFFFF` and `0x7FFF`

@@ -2097,7 +2097,9 @@ static void on_config_file_clicked(GtkWidget *widget, gpointer user_data)

if (g_file_test(global_file, G_FILE_TEST_EXISTS)) g_file_get_contents(global_file, &global_content, NULL, NULL);

- document_new_file(utf8_filename, ft, global_content); + // open or create the document and mark it as changed if it didn't already exist + GeanyDocument *doc = document_new_file(utf8_filename, ft, global_content); + document_set_text_changed(doc, ! g_file_test(file_name, G_FILE_TEST_EXISTS));

Why? saving an empty file, or a copy of the system version, doesn't seem to useful?

@@ -36,6 +36,17 @@

color: #007f00; }

+/* compiler message colors */ +#geany-compiler-error { + color: #ff0000; +} +#geany-compiler-context { + color: #880000;

shouldn't that be `0x7f0000` to be the same as the hardcoded one? Not that it matters too much though, I don't imagine anybody will really see the difference

@@ -50,5 +50,22 @@ widget "*.geany-document-status-changed" style "geany-document-status-changed-st

widget "*.geany-document-status-disk-changed" style "geany-document-status-disk-changed-style" widget "*.geany-document-status-readonly" style "geany-document-status-readonly-style"

+# compiler message colors +style "geany-compiler-error-style" { + fg[NORMAL] = "#ffff00000000" + fg[ACTIVE] = "#ffff00000000" +} +style "geany-compiler-context-style" { + fg[NORMAL] = "#888800000000" + fg[ACTIVE] = "#888800000000"

same pickyness than for the GTK3 variant, for me `65535 / 2` is `7fff`

switch (msg_color)

{ case COLOR_RED: return &color_error; - case COLOR_DARK_RED: return &dark_red; - case COLOR_BLUE: return &blue; + case COLOR_DARK_RED: return &color_context; + case COLOR_BLUE: return &color_message;

We really should rename those color names someday to something semantic. But well.

@@ -83,6 +83,11 @@ enum

};

+static GdkColor color_error = {0, 65535, 0, 0}; +static GdkColor color_context = {0, 65535 / 2, 0, 0}; +static GdkColor color_message = {0, 0, 0, 0xD000};

OK I see it comes from the old code. Well, as you want then, but could be the occasion to make that less odd :)

codebrainz commented on this pull request.

@@ -36,6 +36,17 @@

color: #007f00; }

+/* compiler message colors */ +#geany-compiler-error { + color: #ff0000; +} +#geany-compiler-context { + color: #880000;

This is the commit from #1381, I just imported into this PR.

codebrainz commented on this pull request.

@@ -494,6 +494,27 @@ An example of a simple ``.gtkrc-2.0``::

widget "GeanyPrefsDialog" style "geanyStyle"

+Customizing Geany's appearance using GTK+ 3 CSS +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +To override GTK+ CSS styles, you can use traditional mechanisms or you +can create a file named ``geany.css`` in the user configuration directory +(usually ``~/.config/geany``) which will be loaded after other CSS styles +are applied to allow overriding the default styles. + +Geany offers a number of CSS classes which can be overridden to taylor its

Oops, been too long since I've touched CSS.

codebrainz commented on this pull request.

@@ -1,10 +0,0 @@

-@import "geany.css"; - -/* make close button on the editor's tabs smaller */ -#geany-close-tab-button { - -GtkWidget-focus-padding: 0; - -GtkWidget-focus-line-width: 0; - -GtkButton-default-border: 0; - -GtkButton-default-outside-border: 0; - -GtkButton-inner-border: 0; -}

Using `~/.config/gtk-3.0/gtk.css`, how do you make all the gtkpaned splitters wider or make the statusbar background colour pink, just in Geany without affecting other applications? Using the global gtk.css would only allow you to tweak Geany's named IDs without affecting other applications, any other changes will affect all GTK+ apps.

The only other way is quite a bit complicated, involving custom GTK+ themes applied to Geany alone using environment variables to override the theme, AFAICT.

b4n commented on this pull request.

@@ -2097,7 +2097,9 @@ static void on_config_file_clicked(GtkWidget *widget, gpointer user_data)

if (g_file_test(global_file, G_FILE_TEST_EXISTS)) g_file_get_contents(global_file, &global_content, NULL, NULL);

- document_new_file(utf8_filename, ft, global_content); + // open or create the document and mark it as changed if it didn't already exist + GeanyDocument *doc = document_new_file(utf8_filename, ft, global_content); + document_set_text_changed(doc, ! g_file_test(file_name, G_FILE_TEST_EXISTS));

Well, I don't mind too much, but I don't really see how it's a big deal. But okay if you feel strongly.

codebrainz commented on this pull request.

@@ -1,10 +0,0 @@

-@import "geany.css"; - -/* make close button on the editor's tabs smaller */ -#geany-close-tab-button { - -GtkWidget-focus-padding: 0; - -GtkWidget-focus-line-width: 0; - -GtkButton-default-border: 0; - -GtkButton-default-outside-border: 0; - -GtkButton-inner-border: 0; -}

Ok, now show how you'd make the text in Geany's open dialog larger.

b4n commented on this pull request.

@@ -1,10 +0,0 @@

-@import "geany.css"; - -/* make close button on the editor's tabs smaller */ -#geany-close-tab-button { - -GtkWidget-focus-padding: 0; - -GtkWidget-focus-line-width: 0; - -GtkButton-default-border: 0; - -GtkButton-default-outside-border: 0; - -GtkButton-inner-border: 0; -}

If I wanted to be somewhat unfair, give me a snippet that styles only the file open fialog in all GTK apps :)

codebrainz commented on this pull request.

@@ -1,10 +0,0 @@

-@import "geany.css"; - -/* make close button on the editor's tabs smaller */ -#geany-close-tab-button { - -GtkWidget-focus-padding: 0; - -GtkWidget-focus-line-width: 0; - -GtkButton-default-border: 0; - -GtkButton-default-outside-border: 0; - -GtkButton-inner-border: 0; -}

I don't know specifically about file dialogs with the open file action, since GTK+'s CSS documentation is garbage, I don't even know where to look to find special stuff on a per-widget basis, but to do what I originally meant, style the file chooser for which we have no custom ID, it would just be:

```css GtkFileChooserDialog { font-size: x-large; } ```

codebrainz commented on this pull request.

@@ -1,10 +0,0 @@

-@import "geany.css"; - -/* make close button on the editor's tabs smaller */ -#geany-close-tab-button { - -GtkWidget-focus-padding: 0; - -GtkWidget-focus-line-width: 0; - -GtkButton-default-border: 0; - -GtkButton-default-outside-border: 0; - -GtkButton-inner-border: 0; -}

Lovely, they really like to break stuff in GTK+ don't they :)

(I only tested on my 3.18)

codebrainz commented on this pull request.

@@ -1,10 +0,0 @@

-@import "geany.css"; - -/* make close button on the editor's tabs smaller */ -#geany-close-tab-button { - -GtkWidget-focus-padding: 0; - -GtkWidget-focus-line-width: 0; - -GtkButton-default-border: 0; - -GtkButton-default-outside-border: 0; - -GtkButton-inner-border: 0; -}

It's not really a loss of flexibility, I just saw the opportunity to remove one redundant file. If something else comes up, we can add other CSS files and load them conditionally based on version, like is done for 3.20. The main improvement is that it's now using the Cascading effect instead of relying on `@import`ing another file.

codebrainz commented on this pull request.

@@ -2566,6 +2582,10 @@ void ui_init(void)

ui_init_toolbar_widgets(); init_document_widgets(); create_config_files_menu(); + + // after UI is initialized, apply user's custom CSS (if it exists) + // to override other CSS styles + init_user_style();

Originally I had it all done at the same time but it didn't work. I suspect we setup something in the intervening code that is required. If you're going to test this PR, just try moving `init_user_style` directly under `init_default_style` and it should be reproduced.

codebrainz commented on this pull request.

@@ -2566,6 +2582,10 @@ void ui_init(void)

ui_init_toolbar_widgets(); init_document_widgets(); create_config_files_menu(); + + // after UI is initialized, apply user's custom CSS (if it exists) + // to override other CSS styles + init_user_style();

@b4n ah, right, it was this:

``` Geany-INFO: Using alternate configuration directory Geany-INFO: Geany 1.30 (git >= 30a8921), en_CA.UTF-8 Geany-INFO: GTK 3.18.9, GLib 2.48.2 Geany-INFO: System data dir: /opt/geany/share/geany Geany-INFO: User config dir: /tmp/gtkcsstestconfig Geany-INFO: Loaded GTK+ CSS theme '/opt/geany/share/geany/geany.css'

(geany:27568): Gtk-CRITICAL **: gtk_container_add: assertion 'GTK_IS_CONTAINER (container)' failed Geany-INFO: System plugin path: /opt/geany/lib/geany Geany-INFO: Added filetype JSON (61). Geany-INFO: Added filetype Cython (62). Geany-INFO: Added filetype Scala (63). Geany-INFO: Added filetype CUDA (64). Geany-INFO: Added filetype Genie (65). Geany-INFO: Added filetype Clojure (66). Geany-INFO: Added filetype Graphviz (67). Geany-INFO: Added filetype Arduino (68). Geany-INFO: Loaded libvte from libvte-2.91.so Geany-INFO: unknown : None (UTF-8) ```

I could figure it out, but doing it after fixes this.

b4n commented on this pull request.

@@ -2097,7 +2097,9 @@ static void on_config_file_clicked(GtkWidget *widget, gpointer user_data)

if (g_file_test(global_file, G_FILE_TEST_EXISTS)) g_file_get_contents(global_file, &global_content, NULL, NULL);

- document_new_file(utf8_filename, ft, global_content); + // open or create the document and mark it as changed if it didn't already exist + GeanyDocument *doc = document_new_file(utf8_filename, ft, global_content); + document_set_text_changed(doc, ! g_file_test(file_name, G_FILE_TEST_EXISTS));

For the CSS file, it doesn't matter if it's just a blank file, it gets applied after the system one(s) and if it's blank it will do nothing. […]

The thing is that by default Geany fills up the file with the system version's content. So if you save that, it'll overwrite the system forever. Actually we probably shouldn't suggest to overwrite everything, although it's a fairly good thing to give an example of what the file should contain…

codebrainz commented on this pull request.

@@ -2566,6 +2582,10 @@ void ui_init(void)

ui_init_toolbar_widgets(); init_document_widgets(); create_config_files_menu(); + + // after UI is initialized, apply user's custom CSS (if it exists) + // to override other CSS styles + init_user_style();

I really dislike the way items are added to that menu, a bunch of scattered calls mixed in to all kinds of different modules, each calling that function in a different way. It would be nice to move this to Glade or at least have a single location that populates the menu (which could itself be in glade at least).

codebrainz commented on this pull request.

@@ -2097,7 +2097,9 @@ static void on_config_file_clicked(GtkWidget *widget, gpointer user_data)

if (g_file_test(global_file, G_FILE_TEST_EXISTS)) g_file_get_contents(global_file, &global_content, NULL, NULL);

- document_new_file(utf8_filename, ft, global_content); + // open or create the document and mark it as changed if it didn't already exist + GeanyDocument *doc = document_new_file(utf8_filename, ft, global_content); + document_set_text_changed(doc, ! g_file_test(file_name, G_FILE_TEST_EXISTS));

Ah my bad, I was trying to make it open a new blank CSS file with the correct filename. I agree we shouldn't pre-populate with everything, it'd be better to just put a comment in them like:

Customize this file to override the system default file. See `$prefix/share/geany/whatever.file` for examples of what can go here

codebrainz commented on this pull request.

@@ -2097,7 +2097,9 @@ static void on_config_file_clicked(GtkWidget *widget, gpointer user_data)

if (g_file_test(global_file, G_FILE_TEST_EXISTS)) g_file_get_contents(global_file, &global_content, NULL, NULL);

- document_new_file(utf8_filename, ft, global_content); + // open or create the document and mark it as changed if it didn't already exist + GeanyDocument *doc = document_new_file(utf8_filename, ft, global_content); + document_set_text_changed(doc, ! g_file_test(file_name, G_FILE_TEST_EXISTS));

It's not opening an existing file, it's creating a new unsaved file.

codebrainz commented on this pull request.

@@ -2097,7 +2097,9 @@ static void on_config_file_clicked(GtkWidget *widget, gpointer user_data)

if (g_file_test(global_file, G_FILE_TEST_EXISTS)) g_file_get_contents(global_file, &global_content, NULL, NULL);

- document_new_file(utf8_filename, ft, global_content); + // open or create the document and mark it as changed if it didn't already exist + GeanyDocument *doc = document_new_file(utf8_filename, ft, global_content); + document_set_text_changed(doc, ! g_file_test(file_name, G_FILE_TEST_EXISTS));

You did touch it, you asked Geany to generate you a new file. How does it make sense to have a file open in Geany that has a path, has contents that aren't saved to disk, and yet if you close Geany the file vapourizes into thin air?

codebrainz commented on this pull request.

@@ -2097,7 +2097,9 @@ static void on_config_file_clicked(GtkWidget *widget, gpointer user_data)

if (g_file_test(global_file, G_FILE_TEST_EXISTS)) g_file_get_contents(global_file, &global_content, NULL, NULL);

- document_new_file(utf8_filename, ft, global_content); + // open or create the document and mark it as changed if it didn't already exist + GeanyDocument *doc = document_new_file(utf8_filename, ft, global_content); + document_set_text_changed(doc, ! g_file_test(file_name, G_FILE_TEST_EXISTS));

Maybe I should just take it out of this PR, since it's kind of tangential. Another PR could deal with this and also not filling in the files with all the contents of system files.

b4n commented on this pull request.

@@ -56,8 +56,8 @@ style "geany-compiler-error-style" {

fg[ACTIVE] = "#ffff00000000" } style "geany-compiler-context-style" { - fg[NORMAL] = "#888800000000" - fg[ACTIVE] = "#888800000000" + fg[NORMAL] = "#7f7f00000000" + fg[ACTIVE] = "#7f7f00000000"

Odd choice? AFAIK these are 3 16bits values, not 6 8bits ones. (BTW, AFAIK it's also possible to use the short 8bit values instead of the 16bits ones)

@@ -494,6 +494,27 @@ An example of a simple ``.gtkrc-2.0``::

widget "GeanyPrefsDialog" style "geanyStyle"

+Customizing Geany's appearance using GTK+ 3 CSS +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +To override GTK+ CSS styles, you can use traditional mechanisms or you +can create a file named ``geany.css`` in the user configuration directory +(usually ``~/.config/geany``) which will be loaded after other CSS styles

might be interesting linking http://www.geany.org/manual/#configuration-file-paths

@@ -112,6 +117,40 @@ void msgwin_set_messages_dir(const gchar *messages_dir)

}

+GdkColor load_color(gchar *color_name) {

could this avoid returning an aggregate and "return" through an arg? Yeah I know it's valid C and all, but it's copying at least twice the data (well, 12 instead of 8 bytes on a Linux 64), and it's common to return "by reference" (don't lynch me :]) in such cases. I don't mind *so* much, but I build with `-Waggregate-return` -- and yeah, it *did* spot some meaningful issue once or twice (ok, in other ppl's code generally, but still)

• theme_fn = g_build_filename(app->datadir, "geany.css", NULL);

+ load_css_theme(theme_fn, GTK_STYLE_PROVIDER_PRIORITY_APPLICATION); + g_free(theme_fn); + + // load themes to handle breakage between various GTK+ versions + const struct + { + guint min_version; + guint max_version; + const gchar *file; + } + css_files[] = + { + { 20, G_MAXUINT, "geany-3.20.css" }, + { 0, 19, "geany-3.0.css" }, + };

Any reason why you still change how it's loaded? I don't really mind both seem OK, but AFAIK there's no more point in changing it, and I fairly like the in-CSS flexibility of the `@import`. No biggie though.

codebrainz commented on this pull request.

@@ -56,8 +56,8 @@ style "geany-compiler-error-style" {

fg[ACTIVE] = "#ffff00000000" } style "geany-compiler-context-style" { - fg[NORMAL] = "#888800000000" - fg[ACTIVE] = "#888800000000" + fg[NORMAL] = "#7f7f00000000" + fg[ACTIVE] = "#7f7f00000000"

I just went in python and did `hex(int((0x7f/255.0)*65536))` to scale from 8-bit to 16-bit. Would be better to use 8-bit HTML-style notation, if supported.

codebrainz commented on this pull request.

@@ -112,6 +117,40 @@ void msgwin_set_messages_dir(const gchar *messages_dir)

}

+GdkColor load_color(gchar *color_name) {

I don't mind either way and didn't write this, let me know which way you want.

b4n commented on this pull request.

@@ -112,6 +117,40 @@ void msgwin_set_messages_dir(const gchar *messages_dir)

}

+GdkColor load_color(gchar *color_name) {

I'd like return in parameter, please :)

b4n commented on this pull request.

@@ -56,8 +56,8 @@ style "geany-compiler-error-style" {

fg[ACTIVE] = "#ffff00000000" } style "geany-compiler-context-style" { - fg[NORMAL] = "#888800000000" - fg[ACTIVE] = "#888800000000" + fg[NORMAL] = "#7f7f00000000" + fg[ACTIVE] = "#7f7f00000000"

I just went in python and did `hex(int((0x7f/255.0)*65535))` to scale from 8-bit to 16-bit.

Hence the precision loss :) ```pycon

...

...

hex(int(0xff/2))

'0x7f'

...

...

hex(int(0xffff/2))

'0x7fff'

...

...

hex(int(int(0x7fff*0xff/0xffff)/0xff*0xffff))

'0x7f7f' ```

Would be better to use 8-bit HTML-style notation, if supported.

I think it is supported, just use `"#7f0000"`

HybridDog commented on this pull request.

@@ -56,8 +56,8 @@ style "geany-compiler-error-style" {

fg[ACTIVE] = "#ffff00000000" } style "geany-compiler-context-style" { - fg[NORMAL] = "#888800000000" - fg[ACTIVE] = "#888800000000" + fg[NORMAL] = "#7f7f00000000" + fg[ACTIVE] = "#7f7f00000000"

screens can only display 8 bit truecolor anyway, can't they?