About
case GEANY_KEYS_EDITOR_CONTEXTACTION: - if (check_current_word(doc, FALSE)) + //do not check here: if (check_current_word(doc, FALSE))
Why? I explained this. Without this correction the hotkeying on the empty selection doesn't work. It's not the place to check the access to the function. The function should do it.
On Wed, Apr 25, 2018 at 2:23 AM, elextr notifications@github.com wrote:
*@elextr* requested changes on this pull request.
As said in specific comments, the goto changes should be a separate PR from the context action changes. Other specific comments.
Documentation needed.
In src/build.c https://github.com/geany/geany/pull/1841#discussion_r183905312:
@@ -664,7 +664,8 @@ static void clear_all_errors(void)
/* Replaces occurrences of %e and %p with the appropriate filenames and
- %l with current line number. %d and %p replacements should be in UTF8 */
-static gchar *build_replace_placeholder(const GeanyDocument *doc, const gchar *src) +GEANY_API_SYMBOL
This doesn't need to be in the API, just remove the static.
In src/callbacks.c https://github.com/geany/geany/pull/1841#discussion_r183905945:
@@ -1467,6 +1467,9 @@ void on_context_action1_activate(GtkMenuItem *menuitem, gpointer user_data)
} else {
/* take a current word under caret */IIUC editor_info.current_word is kept updated with the current word, you don't need to find it again.
In src/callbacks.c https://github.com/geany/geany/pull/1841#discussion_r183906397:
@@ -1486,8 +1489,11 @@ void on_context_action1_activate(GtkMenuItem *menuitem, gpointer user_data)
/* substitute the wildcard %s and run the command if it is non empty */ if (G_LIKELY(!EMPTY(command))) {
gchar *command_line = g_strdup(command);
/* add %f, %d, %e, %p, %l placeholders */gchar *command_tmp = g_strdup(command);build_replace_placeholder() does not modify the input string, so no need to copy it.
In src/editor.c https://github.com/geany/geany/pull/1841#discussion_r183906784:
@@ -340,7 +340,23 @@ static gboolean on_editor_button_press_event(GtkWidget *widget, GdkEventButton *
current_word, sizeof current_word, NULL); can_goto = sci_has_selection(editor->sci) || current_word[0] != '\0';
ui_update_popup_goto_items(can_goto);
/* create a list of locks/unlocks for the goto and context actions */gchar cangos[64]; // never reachable maximumfor (gint i=0; i<64; i++)Don't use a manifest constant, use a #defined symbol. And 64 seems silly large.
In src/editor.c https://github.com/geany/geany/pull/1841#discussion_r183906955:
@@ -340,7 +340,23 @@ static gboolean on_editor_button_press_event(GtkWidget *widget, GdkEventButton *
current_word, sizeof current_word, NULL); can_goto = sci_has_selection(editor->sci) || current_word[0] != '\0';
ui_update_popup_goto_items(can_goto);
/* create a list of locks/unlocks for the goto and context actions */gchar cangos[64]; // never reachable maximumfor (gint i=0; i<64; i++)cangos[i]=(can_goto?'1':'0'); // flag the whole groupIf its meant to be a boolean use the gboolean type not characters '1' and '0'.
In src/editor.c https://github.com/geany/geany/pull/1841#discussion_r183907296:
@@ -340,7 +340,23 @@ static gboolean on_editor_button_press_event(GtkWidget *widget, GdkEventButton *
current_word, sizeof current_word, NULL); can_goto = sci_has_selection(editor->sci) || current_word[0] != '\0';
ui_update_popup_goto_items(can_goto);
This whole section fixing/changing the existing goto functionality should be in a separate PR from context action. Don't mix different things in one PR, if one of them is rejected/delayed both will be.
In src/keybindings.c https://github.com/geany/geany/pull/1841#discussion_r183907360:
@@ -2196,7 +2196,7 @@ static gboolean cb_func_editor_action(guint key_id)
editor_show_calltip(doc->editor, -1); break; case GEANY_KEYS_EDITOR_CONTEXTACTION:
if (check_current_word(doc, FALSE))
//do not check here: if (check_current_word(doc, FALSE))Why?
In src/ui_utils.c https://github.com/geany/geany/pull/1841#discussion_r183907690:
{
- guint i, len;
- len = G_N_ELEMENTS(widgets.popup_goto_items);
- for (i = 0; i < len; i++)
ui_widget_set_sensitive(widgets.popup_goto_items[i], enable);
- guint i, len;
- len = G_N_ELEMENTS(widgets.popup_goto_items);
- for (i = 0; i < len; i++)
ui_widget_set_sensitive(widgets.popup_goto_items[i], enable[i]=='1');Indentation is wrong, Geany is tab only indentation. See above comments re boolean and separate PR.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/geany/geany/pull/1841#pullrequestreview-114995112, or mute the thread https://github.com/notifications/unsubscribe-auth/Ajmr5NyTlA-GCxwT3pDesmEsKWTPb3iyks5tr7PigaJpZM4Thz7Y .