There is some problem with Geany's context action.
At first let me cite the announcement of Geany 1.33 release:
_Some highlights: Show status message on attempt to execute empty context action_
Another cite is from an old good English dictionary:
_**context** n. the parts (of anything written) which go before or follow a word, expression or passage and which help to fix its meaning._
Combining these two quotations and applying them to the software we can see that the Geany's context is no context at all. To be more exact, this context is restricted to a selected text of Geany editor.
The real context should include the maximum of Geany's environment (after all, Geany is IDE :), i.e. it should contain:
* a current file name * a current file directory * a current file type * a current file encoding * ... and so on ...
... for other options see "Statusbar Templates" and "Substitutions in commands and working directories" of Geany's help.
Also, Geany's location would be useful in the context because a context handler might require Geany to edit some files.
I try and explain why it's so important to have all this stuff in the Geany's context.
https://wiki.geany.org/howtos/using_with_tcl_tk shows one of many possible implementations of Geany's context processor. And here is its illustrating screenshot:
Please, pay attention to the outlined parts. The fossil commands for processing _e_menu.tcl_ might take out the file name from Geany's context line as _%f wildcard_ if Geany did better. Instead of this, _e_menu.tcl_ (the name of file edited in Geany IDE!) is a literal text hammered into the menu items.
Other outlined parts in the picture demonstrate a usage of %s (current context of Geany) and its stripped of special symbols version. All this is great but far from satisfactory.
Turning to that announced Geany's ignoring the empty context, we can see that it's no highlight.
On the contrary, it's a serious mistake to restrict Geany this way. It's no Geany way.
Shortly, Geany's context has the obvious TODOs: * to include the maximum of Geany's environment into the context string * as consequence, to allow the call of context processor when %s is empty
and (as ideal): * to include %s wildcard into "Compile/ Build/ Run" strings
Probably, it's in no way a hard task as there are similar features in the "Compile/ Build/ Run" settings.
----------------
P.S. A few words about Fossil VCS used in the picture above. It hasn't a direct support in Geany (as Git has) but it's very good for a compact team of developers, say 1 to 10, almost as good as Geany is. So that Fossil and Geany are nearly of the same niche. In some aspects Fossil is better than Git, e.g. its presentation of software history is better than Git's. As well as Fossil's way to roll back and redo back the changes. SQL in all its beauty. Of course it's "imho" only. I employ Git and Fossil on the same project - Git for highlighting the changes in Geany editor after the major commits, Fossil for all the rest.
I always assumed it was called "context action" because it's in the [context menu](https://en.wikipedia.org/wiki/Context_menu).
It seems the following corrections of **src/callback.c** would be sufficient (marked with //apl):
``` void on_context_action1_activate(GtkMenuItem *menuitem, gpointer user_data) <...> /* substitute the wildcard %s and run the command if it is non empty */ //apl if (G_LIKELY(!EMPTY(command))) { gchar *command_line = g_strdup(command); command_line = build_replace_placeholder(doc, command_line); //apl utils_str_replace_all(&command_line, "%s", word); <...> ```
If I understood it right this would execute the current selection as a command. IMHO it could be dangerous to execute the selection without any sanitizing checks. A user could accidentally select the context action. In the good case it causes an error message because the selection does not include a meaningful command. But e.g. if the user is editing a bash script it could also be a ```rm``` command. Although this is not very likely the example points out that it can be dangerous to simply execute the current selection as-is without any checks.
So if this would be changed the question arises if we want to protect the user from the above by e.g. having a config option to enable/disable the selection as a command (in case that the action command is empty) or maybe add a whitelist of commands and only execute the selection if the first word is found in the whitelist.
@aplsimple please use a recognised format like diff. Its not clear what you are actually doing, but it appears that the only change is the addition of a call to `build_replace_placeholder()`. If thats what you wanted why didn't you just say so instead of a novel nobody read?
Also note I think you have a memory leak because you don't free the strdup of command anywhere.
If you correct that and make a proper pull request with the manual documentation matching the change you might have a chance of the change being accepted.
@LarsGit223 certainly if the user set the command setting to `%s` it will run the selection, but thats the case now, and nowhere does Geany attempt to sanitise commands that result from substitution of placeholders into command strings set by users, and nor should it. Even if the command was set to the totally innocuous `echo %s`, a user could select `; rm -rf /` and be in trouble :grin:
@elextr: what I mean was that now the command is stored in ```command``` and nothing will be executed at all if ```command``` is empty. So right now the selection in ```%s``` is not checked but it is only used as command parameters. Therefore there is a certain check because someone needs to have configured the value of ```command```. With the change suggested the risk is different IMHO.
Yes right now someone could select ```; rm -rf /``` but I thought of the risk by having a word selected and accidentally run the context menu. A combination like ```; rm -rf /``` needs to be selected explicitly as it not just a word.
But I am not picky on this, just wanted to point it out.
@LarsGit223 the command is checked for being empty, not for it just being just `%s` which will run the selection (or current word) as the command. But my point is the user has to set it to `%s`, so its clear they want it to run the selection. Its their choice, and not really something Geany should interfere with.
@elextr:
But my point is the user has to set it to %s
Ahh, ok. I did oversee that. Having that in mind it's ok for me to not implement additional checks.
There was another Issue or PR or discussion somewhere about having Geany set environment variables, it seems like that would satisfy this purpose, right? Like whenever Geany updates the status bar it could call a function like (pseudo code):
```c void update_environment(GeanyDocument *doc) { g_setenv("GEANY_CURRENT_FILE", doc->real_path, TRUE); g_setenv("GEANY_CURRENT_LINE", ..., TRUE); g_setenv("GEANY_...", ..., TRUE); ... } ```
Which would work automatically for context action, custom commands and build commands all at once.
Shouldn't need to set Geany's environment, just set them in the spawned env in spawn.c
Well, really setting e.g. in Windows the context action as %s and then calling it on: cmd /c DEL /Q /S /F c:\valuable*.* we would clear the valuable directory off. It would be entirely our mental problem, not Geany's.
Sometimes ago I'd tried the use of GEANY_CURRENT_FILE and other env. variables, without success. Now trying tmp.bat file (as context action tmp.bat %s): echo --------- echo %PATH% echo --------- echo %1 echo --------- echo %GEANY_CURRENT_FILE% echo --------- pause I get: PATH variable - OK Geany selection - OK GEANY_CURRENT_FILE variable - NOT OK i.e. no success again.
Moreover, F5 (Run) gives the same (%s excluding). Geany v.1.29. Windows. (I would test it on other versions and platforms)
Of course noboby need to read my 'novels'.
As for Geany code, I'd tried it closer. The following corrections introduce %f, %d and other Run's wildcards in the context string:
**build.h**
gchar *build_replace_placeholder(const GeanyDocument *doc, const gchar *src);
**build.c**
GEANY_API_SYMBOL gchar *build_replace_placeholder(const GeanyDocument *doc, const gchar *src)
**callbacks.c**
gchar *command_line = g_strdup(command); command_line = build_replace_placeholder(doc, command_line); //apl utils_str_replace_all(&command_line, "%s", word);
But it does not resolve the whole issue.
The modifications below provide: 1. While calling the popup menu: - all find and context menu items are enabled if there is a selected text / caret-set word and there is a defined context action - if there is a defined context action and no selected text/word, the find items are disabled and the context item is enabled - if there is a selected text/word and no defined context action,the find items are enabled and the context item is disabled - if there is no selected text/word and no defined context action, the find items and the context item are disabled 2. While pressing a context hotkey: - if there is a defined context action, the context action is executed 3. The context action string can contain the %f, %d, %e, %p, &l wildcards.
**ui_utils.h**
void ui_update_popup_goto_items(gchar* enable);
**ui_utils.c**
void ui_update_popup_goto_items(gchar* 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'); }
**keybindings.c**
case GEANY_KEYS_EDITOR_CONTEXTACTION: //if (check_current_word(doc, FALSE)) on_context_action1_activate(GTK_MENU_ITEM(ui_lookup_widget(main_widgets.editor_menu, "context_action1")), NULL);
**editor.c**
gchar cangos[64];//never reachable maximum for (gint i=0; i<64; i++) cangos[i]=(can_goto?'1':'0'); //apl gchar* command; if (doc->file_type != NULL && !EMPTY(doc->file_type->context_action_cmd)) command = g_strdup(doc->file_type->context_action_cmd); else command = g_strdup(tool_prefs.context_action_cmd); cangos[1] = (G_LIKELY(!EMPTY(command))?'1':'0'); //flag the context action g_free(command);
ui_update_popup_goto_items(cangos);
**callbacks.c**
{ editor_find_current_word_sciwc(doc->editor, -1, editor_info.current_word, GEANY_MAX_WORD_LENGTH); word = g_strdup(editor_info.current_word); } <..> gchar *command_tmp = g_strdup(command); //== gchar *command_line = build_replace_placeholder(doc, command_tmp); //== g_free(command_tmp); //==
utils_str_replace_all(&command_line, "%s", word);
If you use `git diff` (or the plain `diff` util), the changes will be meaningful to the rest of us. If you provide a pull reques, they will even be actionable.
Thank you so much for the advice.The pull request done. This novel too.
Closed #1836.
github-comments@lists.geany.org