Bash allows more characters besides `[[:alnum:]_]` when declaring function names using the `function` keyword. It also does not require having a pair of parentheses after the name. Some shells may actually implement it differently but we don't have to be that strict since the user explicitly specifies the `function` keyword anyway.
This update implements the ones described above, and also invalidates function names that are completely made up of digits. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/662
-- Commit Summary --
* Enhance detection of sh functions
-- File Changes --
M tagmanager/ctags/sh.c (125)
-- Patch Links --
https://github.com/geany/geany/pull/662.patch https://github.com/geany/geany/pull/662.diff
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/662
I like @konsolebox 's, idea. I vote that his changes be added (as long as the code works).
Thanks for the contribution, @konsolebox . I am new on the team, so it is not up to me to allow the pull-request. Otherwise, I would allow it if I could.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/662#issuecomment-153393847
@b4n any comment on this? Should it be added to "upstream" CTags first?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/662#issuecomment-166051150
See https://github.com/universal-ctags/ctags/pull/729, I submitted an alternative impl that should work fine, is simpler (partly due to how the parser evolved) and seem to touch less additional stuff.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/662#issuecomment-166439748
@b4n: I saw your change to universal-ctags has been merged a long time ago. Can this be closed then?
@LarsGit223 Not yet, apparently we didn't import the change back in Geany yet
Sorry guys I made a mistake here. I was wrong about the idea that only the `function` keyword allows other characters besides `[[:alnum:]_]`. The classic syntax allows it as well. The only significant difference is that the classic syntax conflicts with `extglob`, whereas the `function` keyword does not. When using a syntax like `is_a_number?(){ [[ $1 == +([[:digit:]]) ]]; }`, bash reports `syntax error near unexpected token '}'`. But it doesn't happen when `extglob` is disabled. I probably misinterpreted it as an exception within the classic syntax itself. I never created specially-named functions other than those having the `[X.]f?` or `[X.]f!` format.
Anyway, I still would suggest to have this solution applied. The `function` keyword allows the difference between the special syntax of bash and the classic or POSIX syntax distinguishable. I also don't think people who write POSIX scripts would want to have their function names also validated with special characters which are only recognized in Bash. So generalizing would be bad idea.
P.S. The classic syntax also conflicts with the assignment syntax, but I don't think anyone would want to use the `=` character for its high ambiguity risk, so it's not really a significant argument to use here.
@konsolebox Actually Universal-CTags changed to have the extended syntax as default: universal-ctags/ctags@ec5981f75be07ed827000d06a3d8cc7b8c70ea8d It could be interesting to get your opinion there given the thought you put into it, and possible specific tests etc. If it's not a good idea because it conflicts with other things, it might be a good idea to review it there.
@b4n I interpret that ctags developers prefer everyone to be happy. See https://github.com/universal-ctags/ctags/issues/1261#issuecomment-270820933. Conflicts among syntaxes would have to be ignored for that sake. I'll just choose not to oppose.
This update is probably no longer needed because of universal ctags so closing.
Closed #662.
github-comments@lists.geany.org