Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creating a user defined snippet config file produces a broken file by default #2648

Open
ianmkenney opened this issue Nov 7, 2020 · 8 comments
Labels
bug confirmed A developer reproduced this issue, or it affetcs enough users

Comments

@ianmkenney
Copy link

ianmkenney commented Nov 7, 2020

Geany version: 1.36

Attempting to create a snippets.conf by Tools -> Configuration Files -> snippets.conf brings up a file with comments that are all inverted.

Geany's snippets configuration file
# #
use \n or %newline% for a new line (it will be replaced by the used EOL char(s) - LF, CR/LF, CR).
use \t or %ws% for an indentation step, it will be replaced according to the current document's indent mode.
use \s to force whitespace at beginning or end of a value ('key= value' won't work, use 'key=\svalue').
use %key% for all keys defined in the [Special] section.
use %cursor% to define where the cursor should be placed after completion. You can define multiple
    %cursor% wildcards and use the "Move cursor in snippet" to jump to the next defined cursor
    position in the completed snippet.
You can define a section for each supported filetype to overwrite default settings, the section
name must match exactly the internal filetype name, run 'geany --ft-names' for a full list.
# #
Additionally, you can use most of the template wildcards like {developer}, {command:...},
or {date} in the snippets.
See the documentation for details.

For a list of available filetype names, execute:
geany --ft-names

Default is used for all filetypes and keys can be overwritten by [filetype] sections
# [Default]

special keys to be used in other snippets, cannot be used "standalone"
can be used by %key%, e.g. %brace_open%
nesting of special keys is not supported (e.g. brace_open=\n{\n%brace_close% won't work)
key "wordchars" is very special, it defines the word delimiting characters when looking for
a word to auto complete, leave commented to use the default wordchars
# [Special]
# brace_open=\n{\n\t
# brace_close=}\n
# block=\n{\n\t%cursor%\n}
# block_cursor=\n{\n\t%cursor%\n}
# #wordchars=_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789

Optional keybindings to insert snippets
Note: these can be overridden by Geany's configurable keybindings
# [Keybindings]
# #for=<Ctrl>7

# [C]
# if=if (%cursor%)%block_cursor%
# else=else%block_cursor%
# for=for (i = 0; i < %cursor%; i++)%block_cursor%
# while=while (%cursor%)%block_cursor%
# do=do\n{\n\t%cursor%\n} while (%cursor%)\n
# switch=switch (%cursor%)%brace_open%case %cursor%:\n\t\t%cursor%\n\t\tbreak;\n\tdefault:\n\t\t%cursor%\n%brace_close%

# [C++]
# if=if (%cursor%)%block_cursor%
# else=else%block_cursor%
# for=for (int i = 0; i < %cursor%; i++)%brace_open%\n%brace_close%
# while=while (%cursor%)%block_cursor%
# do=do\n{\n\t%cursor%\n} while (%cursor%)\n
# switch=switch (%cursor%)%brace_open%case %cursor%:\n\t\t%cursor%\n\t\tbreak;\n\tdefault:\n\t\t%cursor%\n%brace_close%
# try=try%block%\ncatch (%cursor%)%block_cursor%

# [Java]
# if=if (%cursor%)%block_cursor%
# else=else%block_cursor%
# for=for (int i = 0; i < %cursor%; i++)%brace_open%\n%brace_close%
# while=while (%cursor%)%block_cursor%
# do=do\n{\n\t%cursor%\n} while (%cursor%)\n
# switch=switch (%cursor%)%brace_open%case %cursor%:\n\t\t%cursor%\n\t\tbreak;\n\tdefault:\n\t\t%cursor%\n%brace_close%
# try=try%block%\ncatch (%cursor%)%block_cursor%

# [PHP]
# if=if (%cursor%)%block_cursor%
# else=else%block_cursor%
# for=for ($i = 0; $i < %cursor%; $i++)%brace_open%\n%brace_close%
# while=while (%cursor%)%block_cursor%
# do=do\n{\n\t%cursor%\n} while (%cursor%)\n
# switch=switch (%cursor%)%brace_open%case %cursor%:\n\t\t%cursor%\n\t\tbreak;\n\tdefault:\n\t\t%cursor%\n%brace_close%
# try=try%block%\ncatch (%cursor%)%block_cursor%

# [Javascript]
# if=if (%cursor%)%block_cursor%
# else=else%block_cursor%
# for=for (i = 0; i < %cursor%; i++)%block_cursor%
# while=while (%cursor%)%block_cursor%
# do=do\n{\n\t%cursor%\n} while (%cursor%)\n
# switch=switch (%cursor%)%brace_open%case %cursor%:\n\t\t%cursor%\n\t\tbreak;\n\tdefault:\n\t\t%cursor%\n%brace_close%
# try=try%block%\ncatch (%cursor%)%block_cursor%

# [C#]
# if=if (%cursor%)%block_cursor%
# else=else%block_cursor%
# for=for (i = 0; i < %cursor%; i++)%block_cursor%
# while=while (%cursor%)%block_cursor%
# do=do\n{\n\t%cursor%\n} while (%cursor%)\n
# switch=switch (%cursor%)%brace_open%case %cursor%:\n\t\t%cursor%\n\t\tbreak;\n\tdefault:\n\t\t%cursor%\n%brace_close%
# try=try%block%\ncatch (%cursor%)%block_cursor%

# [Vala]
# if=if (%cursor%)%block_cursor%
# else=else%block_cursor%
# for=for (i = 0; i < %cursor%; i++)%block_cursor%
# while=while (%cursor%)%block_cursor%
# do=do\n{\n\t%cursor%\n} while (%cursor%)\n
# switch=switch (%cursor%)%brace_open%case %cursor%:\n\t\t%cursor%\n\t\tbreak;\n\tdefault:\n\t\t%cursor%\n%brace_close%
# try=try%block%\ncatch (%cursor%)%block_cursor%

# [ActionScript]
# if=if (%cursor%)%block_cursor%
# else=else%block_cursor%
# for=for (i = 0; i < %cursor%; i++)%block_cursor%
# while=while (%cursor%)%block_cursor%
# do=do\n{\n\t%cursor%\n} while (%cursor%)\n
# switch=switch (%cursor%)%brace_open%case %cursor%:\n\t\t%cursor%\n\t\tbreak;\n\tdefault:\n\t\t%cursor%\n%brace_close%
# try=try%block%\ncatch (%cursor%)%block_cursor%

# [Python]
# for=for i in xrange(%cursor%):\n\t
# if=if %cursor%:\n\t
# elif=elif %cursor%:\n\t
# else=else:\n\t
# while=while %cursor%:\n\t
# try=try:\n\t%cursor%\nexcept Exception, ex:\n\t
# with=with %cursor%:\n\t
# def=def %cursor% (%cursor%):\n\t""" Function doc """\n\t
# class=class %cursor%:\n\t""" Class doc """\n\t\n\tdef __init__ (self):\n\t\t""" Class initialiser """\n\t\tpass

# [Ferite]
# iferr=iferr%block_cursor%fix%block%
# monitor=monitor%block_cursor%handle%block%

# [Haskell]

# [HTML]
# table=<table>\n\t<tr>\n\t\t<td>%cursor%</td>\n\t</tr>\n</table>

# [Erlang]
# case=case %cursor% of\n\t%cursor% -> %cursor%\nend
# if=if\n\t%cursor% -> %cursor%\nend
# begin=begin\n\t%cursor%\nend
# fun=fun(%cursor%) ->\n\t%cursor%\nend
# try=try %cursor% of\n\t%cursor% ->\n\t%cursor%\ncatch\n\t%cursor% ->\n\t%cursor%\nend
# module=-module(%cursor%).
# export=-export(%cursor%).
# compile=-compile(%cursor%).
# include=-include(%cursor%).

This makes it look like comment characters are required to define snippets.

@b4n
Copy link
Member

b4n commented Nov 7, 2020

OK, I confirm this if the comment marker is set to a single space . The default is ~, in which case the issue doesn't appear.

However, there indeed is a bug here; it looks like we're using the toggle comment feature, which has the side effect of possibly uncommenting lines, which is not wanted here.

@b4n b4n added bug confirmed A developer reproduced this issue, or it affetcs enough users labels Nov 7, 2020
@elextr
Copy link
Member

elextr commented Nov 8, 2020

Well the toggle comment feature will only toggle comments with the comment marker in them, hence so long as its not space (as you identified) it will leave existing comments alone and simply comment all the uncommented lines, which is what we want.

IIRC the discussions at the time, toggle comment was used so selected ranges can be uncommented easily in the resulting file without removing the original comments.

But basically if the toggle comment character is set to space the whole toggle comment feature is broken everywhere, not just here. Thats a user problem (most likely a misunderstanding of how "toggle comment" differs from comment/uncomment).

@b4n
Copy link
Member

b4n commented Nov 8, 2020

But basically if the toggle comment character is set to space the whole toggle comment feature is broken everywhere, not just here.

How so? It works fine when triggered, and leads to inserting what looks like a regular comment.
Using a marker that is not usually used allows to use the toggle comment feature to easily disable some part of the code without worrying about regular comments getting uncommented, but if your comments were using the marker you'd have the same problem, and if you didn't select commented lines you wouldn't have any problem either way.

IIRC the discussions at the time, toggle comment was used so selected ranges can be uncommented easily in the resulting file without removing the original comments.

Fair enough, but we should make sure we only ever toggle on, e.g. consider all lines uncommented no matter what.

@elextr
Copy link
Member

elextr commented Nov 8, 2020

How so?

You basically said it in the following sentence, the feature is for toggling blocks of code on/off during development without worrying about internal comments, its not for setting/unsetting comments for permanent use, which is why I made the note about users misunderstanding the feature and breaking it by setting the marker to something common in normal comments, like space.

Perhaps the feature should be renamed so it isn't the same as comment/uncomment in other editors, so users don't assume its the same without understanding it.

make sure we only ever toggle on

Well, in this use that is all we need, so (without looking at how complex it is) we could add an extra parameter to the toggle comment code to limit it to on for uncommented lines only. Or make a separate function, YMMV.

e.g. consider all lines uncommented no matter what.

Won't that double comment comments which is sure to cause issues full of commented comments comments. 😁

@b4n
Copy link
Member

b4n commented Nov 8, 2020

How so?

You basically said it in the following sentence, the feature is for toggling blocks of code on/off during development without worrying about internal comments, its not for setting/unsetting comments for permanent use, which is why I made the note about users misunderstanding the feature and breaking it by setting the marker to something common in normal comments, like space.

Well, IMO that's one possible usage of it (admittedly, the expected one), but it doesn't strike me as "wrong" to use it some other way.

e.g. consider all lines uncommented no matter what.

Won't that double comment comments which is sure to cause issues full of commented comments comments. grin

Well, currently that's the case: lines commented get extra comment prefix, looking like #~ # […], which indeed look kind of odd.

@elextr
Copy link
Member

elextr commented Nov 8, 2020

Well, IMO that's one possible usage of it (admittedly, the expected one), but it doesn't strike me as "wrong" to use it some other way.

Well, its the usage for which the feature is designed, if it is used in unintended ways and doesn't work that is not a bug. And since the feature expects a unique(ish) toggle marker it breaks this config file toggle feature.

Using space for the toggle marker will likely uncomment comments inside the selection, but I guess if thats what the user expects its ok for them, they just have to be careful which lines they select.

Won't that double comment comments which is sure to cause issues full of commented comments comments. 😁

Well, currently that's the case: lines commented get extra comment prefix, looking like #~ # […], which indeed look kind of odd.

Ahh, after RTFC the skipping comments only happens on languages that have multi-line comments so they don't try to nest.

So it basically seems to me that there needs to be specific code for commenting Geany config files (which we know to be .conf filetype even if they are called filetypes.c) and it should comment uncommented lines using the comment marker for .conf files. If that happens to be space then the user just has to be careful selecting lines to uncomment, but they were ok with doing that in their own code, so should be ok with doing it in the config file.

@b4n
Copy link
Member

b4n commented Nov 8, 2020

Agreed. My point was that it's perfectly OK to have whatever marker one on wants, and it shouldn't break the feature here. If it's not convenient for the user is not our problem, it's the user's; but if we let the user select a marker, it should not break anything (or we should forbid it).

And in any case, relying on the marker not creating conflicts is IMO too optimistic for anything but user calling the feature and having a chance to see if it's wrong.

@elextr
Copy link
Member

elextr commented Nov 8, 2020

And in any case, relying on the marker not creating conflicts is IMO too optimistic for anything but user calling the feature and having a chance to see if it's wrong.

Yes, I noticed that there is only one toggle comment marker, not one per filetype as I first thought. Given the increasing popularity of doc comments that use extra characters after the comment token to signify stuff for docomment purposes there is every chance one day different languages will need different toggle markers, but thats another issue, not this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug confirmed A developer reproduced this issue, or it affetcs enough users
Projects
None yet
Development

No branches or pull requests

3 participants