This is my first commit to any open source project so go easy on me. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1060
-- Commit Summary --
* Auto-close brackets if matching bracket found. Fixes #1041.
-- File Changes --
M data/geany.glade (23) M src/editor.c (2) M src/editor.h (1) M src/prefs.c (3)
-- Patch Links --
https://github.com/geany/geany/pull/1060.patch https://github.com/geany/geany/pull/1060.diff
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060
@@ -3479,6 +3496,7 @@ <property name="visible">True</property> <property name="can_focus">True</property> <property name="tooltip_text" translatable="yes">The amount of characters which are necessary to show the symbol autocompletion list</property>
<property name="invisible_char">●</property>
Spurious change. This probably comes form using a different version of Glade than the one we recommend?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060/files/67d9932e2620c252bb9e94d81dcc4...
@@ -3498,6 +3516,7 @@ <property name="visible">True</property> <property name="can_focus">True</property> <property name="tooltip_text" translatable="yes">Display height in rows for the autocompletion list</property>
<property name="invisible_char">●</property>
same (and more below)
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060/files/67d9932e2620c252bb9e94d81dcc4...
@@ -3427,6 +3427,23 @@ </packing> </child> <child>
<object class="GtkCheckButton" id="check_always_auto_close_brackets"><property name="label" translatable="yes">Auto-close brackets if matching bracket found</property>
It probably would be better for it to read more like "Auto-closing bracket even if a matching bracket is found" or something.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060/files/67d9932e2620c252bb9e94d81dcc4...
@@ -3498,6 +3516,7 @@ <property name="visible">True</property> <property name="can_focus">True</property> <property name="tooltip_text" translatable="yes">Display height in rows for the autocompletion list</property>
<property name="invisible_char">●</property>
Yea I noticed that too. I am using Glade 3.8.5 though.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060/files/67d9932e2620c252bb9e94d81dcc4...
@@ -1108,6 +1108,9 @@ on_prefs_dialog_response(GtkDialog *dialog, gint response, gpointer user_data)
widget = ui_lookup_widget(ui_widgets.prefs_dialog, "check_symbol_auto_completion"); editor_prefs.auto_complete_symbols = gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(widget));
widget = ui_lookup_widget(ui_widgets.prefs_dialog, "check_always_auto_close_brackets");editor_prefs.always_auto_close_brackets = gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(widget));
the setting is never loaded. Also, new settings should use Stash API, and here probably be in *keyfile.c:init_pref_groups()*.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060/files/67d9932e2620c252bb9e94d81dcc4...
@@ -3427,6 +3427,23 @@ </packing> </child> <child>
<object class="GtkCheckButton" id="check_always_auto_close_brackets"><property name="label" translatable="yes">Auto-close brackets if matching bracket found</property>
I tried a while to think of a line that wouldn't be too long. I can change it though.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060/files/67d9932e2620c252bb9e94d81dcc4...
As said on the issue it fixes, I don't like "improving" the builting auto-close feature because IMO it's totally broken and useless, and there are plugins doing this better already available. Also, although the "fix" makes some sense, IMO it just shows how broken the current approach is: it tries not to insert inappropriate pairs, but doesn't do it well enough.
Anyway, if really people want, I guess something like that would be fine-ish.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060#issuecomment-224969954
@@ -3427,6 +3427,23 @@ </packing> </child> <child>
<object class="GtkCheckButton" id="check_always_auto_close_brackets"><property name="label" translatable="yes">Auto-close brackets if matching bracket found</property>
Maybe add a tooltip if the label isn't enough to be clear
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060/files/67d9932e2620c252bb9e94d81dcc4...
@@ -3498,6 +3516,7 @@ <property name="visible">True</property> <property name="can_focus">True</property> <property name="tooltip_text" translatable="yes">Display height in rows for the autocompletion list</property>
<property name="invisible_char">●</property>
Odd. But don't add it to the patch though, we'll see what's what later, but that's noise here.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060/files/67d9932e2620c252bb9e94d81dcc4...
@@ -3427,6 +3427,23 @@ </packing> </child> <child>
<object class="GtkCheckButton" id="check_always_auto_close_brackets"><property name="label" translatable="yes">Auto-close brackets if matching bracket found</property>
What about simply "Always auto-close brackets"?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060/files/67d9932e2620c252bb9e94d81dcc4...
@@ -3498,6 +3516,7 @@ <property name="visible">True</property> <property name="can_focus">True</property> <property name="tooltip_text" translatable="yes">Display height in rows for the autocompletion list</property>
<property name="invisible_char">●</property>
-1, better to add a few noise lines than to manually edit the glade file and push the problem onto the next guy like the problems we had before, IMO. It's just a generated file anyway, not like it's actual code people should be reading :)
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060/files/67d9932e2620c252bb9e94d81dcc4...
@@ -3498,6 +3516,7 @@ <property name="visible">True</property> <property name="can_focus">True</property> <property name="tooltip_text" translatable="yes">Display height in rows for the autocompletion list</property>
<property name="invisible_char">●</property>
I don't think I had such issues last time I modified that file, so IMO it requires at least an investigation. Not to add and remove that every now and then.
Sure it's generated, but it requires reviewing as it's the source file. Less noise means less burden on the review.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060/files/67d9932e2620c252bb9e94d81dcc4...
@@ -3498,6 +3516,7 @@ <property name="visible">True</property> <property name="can_focus">True</property> <property name="tooltip_text" translatable="yes">Display height in rows for the autocompletion list</property>
<property name="invisible_char">●</property>
I just tried sticking a checkbox in the same place and it doesn't cause any extra noise, FWIW. Using custom built Glade 3.8.5 on Ubuntu 15.10.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060/files/67d9932e2620c252bb9e94d81dcc4...
@@ -3498,6 +3516,7 @@ <property name="visible">True</property> <property name="can_focus">True</property> <property name="tooltip_text" translatable="yes">Display height in rows for the autocompletion list</property>
<property name="invisible_char">●</property>
I'm using Glade 3.8.5, or glade-gtk2, on Arch Linux. Anyways, the invisible character used seems to depend on the current font you're using. Source code comments confirm this. https://github.com/GNOME/gtk/blob/gtk-2-24/gtk/gtkentry.c#L6907
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060/files/67d9932e2620c252bb9e94d81dcc4...
@@ -3427,6 +3427,23 @@ </packing> </child> <child>
<object class="GtkCheckButton" id="check_always_auto_close_brackets"><property name="label" translatable="yes">Auto-close brackets if matching bracket found</property>
Was thinking about that one actually. I think I'll use that and add a tooltip.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060/files/67d9932e2620c252bb9e94d81dcc4...
Alright, I used the Stash API, changed the label and tooltip, and changed the invisible character. I went ahead and amended my current commit instead of using a new one if that's alright. Just tell me if there are any problems.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1060#issuecomment-225114224
github-comments@lists.geany.org