Re: [Github-comments] [geany/geany] Better snippets (#1470)
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 Tab otherwise), this function probably
shouldn't beep here. It's pretty annoying to get a beep every time you press
Tab (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
--
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/1470#pullrequestreview-51674789
Re: [Github-comments] [geany/geany] Better snippets (#1470)
\o/ -- 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/1470#issuecomment-316927995
Re: [Github-comments] [geany/geany] Better snippets (#1470)
Merged #1470. -- 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/1470#event-1173353996
Re: [Github-comments] [geany/geany] Better snippets (#1470)
LGBI, and tested by @vfaronov, thanks, merged. -- 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/1470#issuecomment-316918337
Re: [Github-comments] [geany/geany] Better snippets (#1470)
Yes, everything seems fixed to me. -- 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/1470#issuecomment-314164286
Re: [Github-comments] [geany/geany] Better snippets (#1470)
Should be fixed as well. -- 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/1470#issuecomment-314132567
Re: [Github-comments] [geany/geany] Better snippets (#1470)
@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. -- 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/1470#issuecomment-312424847
Re: [Github-comments] [geany/geany] Better snippets (#1470)
@vfaronov Thanks for testing. I think I addressed both issues. Please give it one more try. -- 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/1470#issuecomment-311684202
Re: [Github-comments] [geany/geany] Better snippets (#1470)
@kugel- pushed 2 commits. 65e283e snippts: use ascii character for the placeholder. 4e2cb02 snippets: fix EOF detection, when searching for the next cursor -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1470/files/07c0e1ce75f1f96a859b8ebb7fca67b3274d566e..4e2cb028c2bea658d5bb6533db0253319c756552
Re: [Github-comments] [geany/geany] Better snippets (#1470)
@kugel- maybe use an ASCII space with one of the box indicators on it, that might stand out well enough? -- 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/1470#issuecomment-310965670
Re: [Github-comments] [geany/geany] Better snippets (#1470)
OTOH, I probably shouldn't be inserting utf8 chars into non-utf8 docs. -- 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/1470#issuecomment-310963765
Re: [Github-comments] [geany/geany] Better snippets (#1470)
@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. -- 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/1470#issuecomment-310937877
Re: [Github-comments] [geany/geany] Better snippets (#1470)
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-11e7-9469-33dff37993de.png)), 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... -- 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/1470#issuecomment-310904950
Re: [Github-comments] [geany/geany] Better snippets (#1470)
@elextr ping -- 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/1470#issuecomment-301019094
Re: [Github-comments] [geany/geany] Better snippets (#1470)
@elextr The commit message of 8e9123e explains it all. What's unclear? -- 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/1470#issuecomment-298193736
Re: [Github-comments] [geany/geany] Better snippets (#1470)
@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? -- 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/1470#issuecomment-298166100
Re: [Github-comments] [geany/geany] Better snippets (#1470)
Yes, fixed for me. -- 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/1470#issuecomment-298163002
Re: [Github-comments] [geany/geany] Better snippets (#1470)
@vfaronov should be fixed (I think I was on a wrong branch when I tried to reproduce the problem). -- 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/1470#issuecomment-298162544
Re: [Github-comments] [geany/geany] Better snippets (#1470)
Actually, I cannot reproduce the behavior change. A snippet "asd=asdfoo" inserts adds foo on asd and the cursor position goes to the end. Can you show me your snippet config? -- 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/1470#issuecomment-298108794
Re: [Github-comments] [geany/geany] Better snippets (#1470)
This was not intentional, I'll fix that. Thanks for testing. -- 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/1470#issuecomment-298105868
Re: [Github-comments] [geany/geany] Better snippets (#1470)
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 ;) -- 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/1470#issuecomment-297007937
[Github-comments] [geany/geany] Better snippets (#1470)
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.  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 -- 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/1470
