| Summary: | Version 2.15 of bash syntax highlighting is incorrect | ||
|---|---|---|---|
| Product: | [Applications] kate | Reporter: | necrodust44 |
| Component: | syntax | Assignee: | KWrite Developers <kwrite-bugs-null> |
| Status: | RESOLVED FIXED | ||
| Severity: | minor | CC: | luizluca |
| Priority: | NOR | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | unspecified | ||
| OS: | Linux | ||
| Latest Commit: | http://commits.kde.org/kate/b9947e0454dbb3b9d11be5a6ffb8331569d8df31 | Version Fixed In: | 4.10 |
| Bug Depends on: | 140511 | ||
| Bug Blocks: | |||
| Attachments: |
This patch fixes the regression introduced in version 2.14: backquote and parenthesis on command arguments
This is the bash.xml after patch applied |
||
|
Description
necrodust44
2012-12-06 00:12:32 UTC
Luiz, the patch you provided in bug #140511 seems to have regressions. Can you have a look? Otherwise, we'll have to revert your changes for KDE 4.10 again. @necrodust44: > But the primary question is who adopted these changes? Now I can take any syntax script, > change it so that it will produce incorrect syntax highlighting, upload it to the repositories, > and nobody will even care? We have > 200 highlightings, it is impossible to test all corner cases, especially since we cannot know /all/ the languages out there. So instead of flaming here, provide a patch to fix it. This helps much more. Thanks for you bug report, though, since this is the correct way to get things fixed :) I'll take a look In the last patch, in order to avoid highlighting special names as arguments like in: echo do the job I splitted the command processing into "FindCommand" and "CommandArgs". However, I missed the backquote case (that was reported) and the parenthesis case (still not reported). As I use a subcontext (CommandArgs), I must create CommandArgs context for each of the special situations (backquote and parenthesis), just like comment already does. I would be much easier if kate could pass someway information to lower context, so I could use only one context for all three situations. I'm finishing the parenthesis part and I'll post the patch soon I think that you should test you version more thoroughly before you add it as stable. And the backquotes and parentheses seem to be not the only cases. For example, I noticed that long arguments like --argument=value were treated as variables and the left half was being marked. necrodust44,
Syntax highlight is heuristic. The current katepart does not allow or it is too much complicated to match every single detail of languages syntax. If you check my bugs about kate syntax highlight, so would see a bunch of cases where it fails. For example:
echo $((echo xxx) )
The last time I checked, even inside bash own parser, this case is a hack. You will only know when it is "$((xxx))" of "$( (xxx) )" after the final parenthesis is matched. However, at that time, "$((" and all its contents were already consumed.
Now back to the real problem. I improved the command parsing and now I detect not known commands (i.e.: not builtin and unixcommands) and I added a new color called OtherCommands. This way, CommandArgs can be selected correctly for all my test cases. I also pop the context, without consuming the text, when it finds a "}" or ")". This will fix the parenthesis problem.
For the backquote, it is a different beast. I need to know if backquote was already open and the found backquote closes it or if it opens a new one. The solution is to create a parallel context for each case, sharing most of its rules but differing when they find the backquote. This seems to do the trick, at least for your grub scripts.
Personally, I never ever use backquote. The syntax "$(cmd)" does just the same and it also allow to place it inside another one easily "$(cmd $(cmd2))".
The next time you face a syntax problem with bash, do not insult the contributors via personal emails. This does not helps. Generally it is a very bad idea to insult the one that you need help from. Remember that this is volunteer work. Also, try to isolate the problem, place more information in the bug report as a small script with the problem. You may be new to this "world" but this is how things work here.
Created attachment 75713 [details]
This patch fixes the regression introduced in version 2.14: backquote and parenthesis on command arguments
This patch fixes the regression when "`", "}", ")" closes when inside the command argument context. Also, it now highlight unknown command when it is at the command position.
The patch also cleans some xml ident.
Can you guys test it? I had no problem with the "--param=xxx" situation.
Created attachment 75714 [details]
This is the bash.xml after patch applied
In order to test, just place this file at:
~/.kde4/share/apps/katepart/syntax/bash.xml
@necrodust44: can you quickly test this, please? If there are no regressions to version 2.14, it's fine to commit. the file attached here fixes a serious regression for me (i.e.: foo="bla $(pwd) asdf" would never exit the string highlighting context). can't see any other regressions from a quick glance at some of my scripts. Git commit 5c905a445476032d816e4e1db070c3e52a523bda by Dominik Haumann. Committed on 28/12/2012 at 12:33. Pushed by dhaumann into branch 'master'. fix regression of bash version 2.15 Thanks to Luiz Angelo De Luca for the fix! Does this also fix 312121 and 312274 ? Related: bug 312121, bug 312274 FIXED-IN: 4.10 M +62 -29 part/syntax/data/bash.xml http://commits.kde.org/kate/5c905a445476032d816e4e1db070c3e52a523bda Git commit b9947e0454dbb3b9d11be5a6ffb8331569d8df31 by Dominik Haumann. Committed on 28/12/2012 at 12:33. Pushed by dhaumann into branch 'KDE/4.10'. fix regression of bash version 2.15 Thanks to Luiz Angelo De Luca for the fix! Does this also fix 312121 and 312274 ? Related: bug 312121, bug 312274 FIXED-IN: 4.10 M +62 -29 part/syntax/data/bash.xml http://commits.kde.org/kate/b9947e0454dbb3b9d11be5a6ffb8331569d8df31 Thanks Dominik |