Lots of improvements to snippets.
The end result is shown on the screen shot, and you can use Tab to go to the next cursor position.
![grafik](https://cloud.githubusercontent.com/assets/564520/25215453/db96d138-259d-11e...)
You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1470
-- Commit Summary --
* snippets: Allow keybinding overloading of snippet-next-cursor. * snippets: Remove cursor position at the end of constructs. * snippets: Use Scintilla indicators for cursor posititons. * api: Increment API version.
-- File Changes --
M data/snippets.conf (34) M src/editor.c (168) M src/editor.h (5) M src/highlighting.c (5) M src/keybindings.c (4) M src/plugindata.h (2)
-- Patch Links --
https://github.com/geany/geany/pull/1470.patch https://github.com/geany/geany/pull/1470.diff
Before this PR, when I insert a snippet that doesn’t have any `%cursor%` positions, the cursor goes to the end of the inserted text. This is convenient for inserting short static strings. For example, I have incantations for invoking a debugger at a given point in the code.
This PR breaks this behavior: the cursor stays where it was, and text is just inserted after it.
IMHO when inserting something without explicit cursor, it should jump to the end — but to be honest didn't had a look at the PR at all by now ;)
This was not intentional, I'll fix that. Thanks for testing.
Actually, I cannot reproduce the behavior change. A snippet "asd=asdfoo" inserts adds foo on asd<tab> and the cursor position goes to the end. Can you show me your snippet config?
The default Geany config already includes one such snippet — `else` for Python. So I can reproduce the problem as follows:
1. Start Geany with a pristine config (`geany -c /some/empty/dir`). 2. Open a Python file. 3. Type `else` somewhere in the code and press Tab.
The snippet expands to `else:`, then newline, then indentation. On master, the cursor is then placed after the indentation, but with your branch merged in, it *jumps* to the beginning of the word `else` (not just stays in place — I was mistaken about that).
@kugel- pushed 1 commit.
bc0a329 snippts: restore behavior of cursor-less snippets
@vfaronov should be fixed (I think I was on a wrong branch when I tried to reproduce the problem).
Yes, fixed for me.
@kugel- You have removed the `%cursor%` markers from the ends of default snippets, why, and is it something we need to tell users to do to their own snippets after this change?
@elextr The commit message of 8e9123e explains it all. What's unclear?
@elextr ping
@kugel- that explanation is adequate, thank you.
I have been running with this PR for some time. I’m not a heavy user of snippets, but I do use a few.
I also tested this PR specifically, under Linux GTK+2, Linux GTK+3, and Windows GTK+2. Things like:
* inserting various snippets, jumping around them; * completing a snippet inside another snippet; * completing a snippet inside a compiler error (to see how the two indicators interact); * invoking editing operations (like deleting an entire line) while inside a snippet expansion.
--------------------------------------------------
**Bug:** when the cursor is on *or after* the last cursor position (ellipsis), pressing the “Move cursor in snippet” keybinding moves the cursor to the end of the document. In more detail:
1. Open a non-empty Python document. 2. Put the cursor somewhere, type `for`, and press the “Complete snippet” keybinding. The snippet expands, and the cursor jumps to inside the parentheses in `xrange(…)`. 3. Press the “Move cursor in snippet” keybinding. 4. On master, the cursor stays in place. But with this PR, the cursor jumps to the end of the document.
--------------------------------------------------
**Bug:** when the cursor is at the beginning of the document, and there’s an ellipsis right there, pressing the “Move cursor in snippet” keybinding skips that ellipsis and selects the following run of text instead. In more detail:
1. Define a snippet like this:
[Default] test=%cursor% something
2. Open an empty document. 3. Type `test` and press the “Complete snippet” keybinding. The snippet expands to `… something`. 4. Press Home to move the cursor to the beginning. 5. Press the “Move cursor in snippet” keybinding. The `…` should be selected, but instead ` something` is selected.
--------------------------------------------------
Consider the main change this PR brings. Previously, cursor positions were invisible and transient. Now, they are marked with an ellipsis (as can be seen on [Thomas’s screenshot](https://cloud.githubusercontent.com/assets/564520/25215453/db96d138-259d-11e...)), which is *literally* inserted into the document. It looks nice, but might have unintended side effects.
Try this:
1. Open an empty (but existing) Python file. 2. Type `def` and press the “Complete snippet” keybinding. The document now contains:
def … (…): """ Function doc """
3. On Windows, this “function” actually appears in the sidebar’s Symbols pane, with a placeholder name that looks like ⛝, and at the same time, a warning is printed to the console:
Pango: Invalid UTF-8 string passed to pango_layout_set_text()
4. On Linux, nothing appears in the Symbols pane, but another warning is printed:
geany: Warning: ignoring null tag in /home/vasiliy/tmp/untitled.py
This may or may not be a problem. You could as well type funny Unicode stuff manually into the document...
@vfaronov your second bug is interesting (and not really the PRs fault I don't think) if a character is displayable in the editor it should be displayable in the symbols pane, so maybe tagmanager or the parser is mishandling UTF-8 characters so its not actually the same string (though I thought that tagmanager at least had been fixed). And parsers must handle illegal syntax and characters since, as you point out, they can occur when users are typing, but bugs may still exist.
Interesting that its different between Win and Lin, are they running with the same locale? I'm thinking C character classification functions that are locale dependent.
OTOH, I probably shouldn't be inserting utf8 chars into non-utf8 docs.
Yes, it might cause a problem if a file is saved in an encoding that doesn't contain that character, but it shouldn't cause problems in memory, both the symbols pane and the buffer are UTF-8.
@kugel- maybe use an ASCII space with one of the box indicators on it, that might stand out well enough?
@kugel- pushed 2 commits.
65e283e snippts: use ascii character for the placeholder. 4e2cb02 snippets: fix EOF detection, when searching for the next cursor
@vfaronov Thanks for testing. I think I addressed both issues. Please give it one more try.
@kugel- This has not been addressed:
**Bug:** when the cursor is at the beginning of the document, and there’s an ellipsis right there, pressing the “Move cursor in snippet” keybinding skips that ellipsis and selects the following run of text instead.
Should be fixed as well.
Yes, everything seems fixed to me.
Merged #1470.
LGBI, and tested by @vfaronov, thanks, merged.
\o/
b4n commented on this pull request.
After quick testing I like the new behavior and the use of indicators :) I'm not sure I like not being able to jump at the end of several snippets (like `def` for Python, which IMO would be handier with a `%cursor%` at the end), but it might require some good testing for pros and cons.
The uninitialized var issue might be serious though. See #1554.
g_string_free(buf, TRUE);
}
+static gboolean find_next_snippet_indicator(GeanyEditor *editor, SelectionRange *sel) +{ + ScintillaObject *sci = editor->sci; + gint val;
unused variable (see 871e72cc03c2d9501a54652ebeb0ca15acf14a20)
+{
+ Sci_Position start, len; +} SelectionRange; + + +#define CURSOR_PLACEHOLDER "_" /* Would rather use … but not all docs are unicode */ +/* Replaces the internal cursor markers with the placeholder suitable for + * display. Except for the first cursor if indicator_for_first is FALSE, + * which is simply deleted. + * + * Returns insertion points as SelectionRange list, so that the caller + * can use the positions (currently for indicators). */ +static GSList *replace_cursor_markers(GeanyEditor *editor, GString *template, + gboolean indicator_for_first) +{ + gssize cur_index = -1;
Unused variable (871e72cc03c2d9501a54652ebeb0ca15acf14a20)
+ +#define CURSOR_PLACEHOLDER "_" /* Would rather use … but not all docs are unicode */ +/* Replaces the internal cursor markers with the placeholder suitable for + * display. Except for the first cursor if indicator_for_first is FALSE, + * which is simply deleted. + * + * Returns insertion points as SelectionRange list, so that the caller + * can use the positions (currently for indicators). */ +static GSList *replace_cursor_markers(GeanyEditor *editor, GString *template, + gboolean indicator_for_first) +{ + gssize cur_index = -1; + gint i = 0; + GSList *temp_list = NULL; + gint cursor_steps = 0, old_cursor = 0;
`old_cursor` is unused (871e72cc03c2d9501a54652ebeb0ca15acf14a20)
/* Move the cursor to the next specified cursor position in an inserted snippet.
* Can, and should, be optimized to give better results */ -void editor_goto_next_snippet_cursor(GeanyEditor *editor) +gboolean editor_goto_next_snippet_cursor(GeanyEditor *editor) { ScintillaObject *sci = editor->sci; gint current_pos = sci_get_current_position(sci);
This is now unused (871e72cc03c2d9501a54652ebeb0ca15acf14a20)
- snippet_cursor_insert_pos = sci_get_current_position(sci); + /* Set cursor to the requested index, or by default to after the snippet */ + if (cursor_index >= 0) + sci_set_current_position(sci, insert_pos + idx, FALSE);
`idx` can be left uninitialized if both `newline_indent_size` and `cursor_index` are not `-1`. I tried to fix it in 734e0604254bcb9f12285c54309e7e71394f75c8, but I'm not 100% certain initializing to `0` is correct in all other cases.
@@ -2394,6 +2388,50 @@ static void fix_indentation(GeanyEditor *editor, GString *buf)
}
+typedef struct +{ + Sci_Position start, len; +} SelectionRange; + + +#define CURSOR_PLACEHOLDER "_" /* Would rather use … but not all docs are unicode */
Why not use `...` (ASCII ellipsis)? I tried to do that in 957b49b868214a4aea81641dedeabb98a497e4c1 and it seems to work great, and looks better to me.
{
- gint offset; - - offset = GPOINTER_TO_INT(g_queue_pop_head(snippet_offsets)); - if (current_pos > snippet_cursor_insert_pos) - snippet_cursor_insert_pos = offset + current_pos; - else - snippet_cursor_insert_pos += offset; - - sci_set_current_position(sci, snippet_cursor_insert_pos, TRUE); + sci_indicator_set(sci, GEANY_INDICATOR_SNIPPET); + sci_set_selection(sci, sel.start, sel.start + sel.len); + return TRUE; } else { utils_beep();
Now the keybinding can be shared (and IIUC that's your indention, and it seems to work pretty nicely with <kbd>Tab</kbd> otherwise), this function probably shouldn't beep here. It's pretty annoying to get a beep every time you press <kbd>Tab</kbd> (if bound to this). Ideally I guess it would beep if there's no other handling of the keybinding, but that's not really something that we can do without huge amounts of hackery -- which is not worth it.
Done in dcea941af17b4ede20d4da87ac60cf90efd82448
github-comments@lists.geany.org