<p><b>@elextr</b> requested changes on this pull request.</p>
<p>As said in specific comments, the goto changes should be a separate PR from the context action changes. Other specific comments.</p>
<p>Documentation needed.</p><hr>
<p>In <a href="https://github.com/geany/geany/pull/1841#discussion_r183905312">src/build.c</a>:</p>
<pre style='color:#555'>> @@ -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
</pre>
<p>This doesn't need to be in the API, just remove the static.</p>
<hr>
<p>In <a href="https://github.com/geany/geany/pull/1841#discussion_r183905945">src/callbacks.c</a>:</p>
<pre style='color:#555'>> @@ -1467,6 +1467,9 @@ void on_context_action1_activate(GtkMenuItem *menuitem, gpointer user_data)
}
else
{
+ /* take a current word under caret */
</pre>
<p>IIUC <code>editor_info.current_word</code> is kept updated with the current word, you don't need to find it again.</p>
<hr>
<p>In <a href="https://github.com/geany/geany/pull/1841#discussion_r183906397">src/callbacks.c</a>:</p>
<pre style='color:#555'>> @@ -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);
</pre>
<p><code>build_replace_placeholder()</code> does not modify the input string, so no need to copy it.</p>
<hr>
<p>In <a href="https://github.com/geany/geany/pull/1841#discussion_r183906784">src/editor.c</a>:</p>
<pre style='color:#555'>> @@ -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 maximum
+ for (gint i=0; i<64; i++)
</pre>
<p>Don't use a manifest constant, use a <code>#define</code>d symbol. And 64 seems silly large.</p>
<hr>
<p>In <a href="https://github.com/geany/geany/pull/1841#discussion_r183906955">src/editor.c</a>:</p>
<pre style='color:#555'>> @@ -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 maximum
+ for (gint i=0; i<64; i++)
+ cangos[i]=(can_goto?'1':'0'); // flag the whole group
</pre>
<p>If its meant to be a boolean use the <code>gboolean</code> type not characters '1' and '0'.</p>
<hr>
<p>In <a href="https://github.com/geany/geany/pull/1841#discussion_r183907296">src/editor.c</a>:</p>
<pre style='color:#555'>> @@ -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);
+
+
</pre>
<p>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.</p>
<hr>
<p>In <a href="https://github.com/geany/geany/pull/1841#discussion_r183907360">src/keybindings.c</a>:</p>
<pre style='color:#555'>> @@ -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))
</pre>
<p>Why?</p>
<hr>
<p>In <a href="https://github.com/geany/geany/pull/1841#discussion_r183907690">src/ui_utils.c</a>:</p>
<pre style='color:#555'>> {
- 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');
</pre>
<p>Indentation is wrong, Geany is tab only indentation. See above comments re boolean and separate PR.</p>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/geany/geany/pull/1841#pullrequestreview-114995112">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABDrJ0eVLn0vBoBkvLP9BJE5f2sjBWp5ks5tr7PggaJpZM4Thz7Y">mute the thread</a>.<img src="https://github.com/notifications/beacon/ABDrJxd3ZSJPiXY2YXdmChl8ViXgR_mVks5tr7PggaJpZM4Thz7Y.gif" height="1" width="1" alt="" /></p>
<div itemscope itemtype="http://schema.org/EmailMessage">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
<link itemprop="url" href="https://github.com/geany/geany/pull/1841#pullrequestreview-114995112"></link>
<meta itemprop="name" content="View Pull Request"></meta>
</div>
<meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/geany/geany","title":"geany/geany","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/geany/geany"}},"updates":{"snippets":[{"icon":"PERSON","message":"@elextr requested changes on #1841"}],"action":{"name":"View Pull Request","url":"https://github.com/geany/geany/pull/1841#pullrequestreview-114995112"}}}</script>