Bug 103756 - kate: syntax highlighting error: bash and escaped quotes
Summary: kate: syntax highlighting error: bash and escaped quotes
Status: RESOLVED WORKSFORME
Alias: None
Product: kate
Classification: Applications
Component: syntax (show other bugs)
Version: unspecified
Platform: Mandrake RPMs Linux
: NOR wishlist
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords: triaged
Depends on:
Blocks:
 
Reported: 2005-04-12 23:23 UTC by Richard Neill
Modified: 2018-10-27 04:14 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
bash.xml.diff (948 bytes, patch)
2007-03-31 01:53 UTC, Carsten Lohrke
Details
My current bash.xml (37.39 KB, text/plain)
2007-04-13 17:29 UTC, Matthew Woehlke
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Neill 2005-04-12 23:23:48 UTC
Version:            (using KDE KDE 3.3.2)
Installed from:    Mandrake RPMs
OS:                Linux

The following line of code completely messes up Kate's sytax highlighting: it makes kate believe that everything that follows it for the rest of the file is a doublequoted string. I am using the 2.02 version of syntax highlighting for Bash, which is currently the latest. Here is the line:

 FILENAME[$i]="`echo -n ${ALBUM}_-_${TRACK2DIGITNO[$i]}_-_${TRACKNAME[$i]} | tr -d \"'\\\"\" | tr -c '[:alnum:]-_=+()[]@#~' '_' | tr -s '_' | sed 's/_$//'`.mp3"


Yes, the line is correct: it does what it's supposed to do! A simpler case is this one:

 xxx="`echo -n $xxx | tr -d \"'\\\"\"`.mp3"

[And yes, writing something like that really should have made me do it in perl!]

Thanks very much - Richard
Comment 1 Rick 2005-06-28 17:14:58 UTC
Confirmed kate version 2.3.1 though it was also present in the version and update released for Redhat 7.3
Comment 2 Christoph Cullmann 2005-09-29 20:28:18 UTC
yes, hl is not perfect for bash, but this is a wish ;)
Comment 3 Juuso Alasuutari 2006-02-22 21:50:32 UTC
I confirm this. This simple line is enough to screw up bash script highlighting:

VAR="${VAR//\"/}"

So far I've solved the problem by adding a commented double quote after such lines:

# "

This normalizes highlighting from there on.

The kate version I use is 2.5.1.
Comment 4 Wilbert Berendsen 2006-02-25 22:37:40 UTC
I see the problem: Escapes inside ${ ... } are not checked. I can see two solutions: add a line <IncludeRules context="FindOthers"/> to the VarBrace context, or just add a line <RegExpr attribute="Escape" context="#stay" String="\\[][;&quot;\\'$`{}()|&amp;&lt;&gt;* ]" /> to that same context.

The last one is probably the best one.
Comment 5 Wilbert Berendsen 2006-02-25 23:00:17 UTC
I think these are actually two little bugs, the one I mentioned in my previous message and another one:

(a) xxx="`echo -n $xxx | tr -d \"'\\\"\"`.mp3" 

is not highlighted correctly but

(b) xxx="`echo -n $xxx | tr -d "'\\\""`.mp3"

is. Is it true that Bash completely parses the
"`echo -n $xxx | tr -d \"'\\\"\"`.mp3"
construct before calling the backtick substitution?

Currently the highlighter just enters the StringDQ (string double quote) context when a " is found. Then it calls the SubstBackq as soons as a the ` is found. Inside the SubstBackq context it may enter a new string (namely "'\\\""). The highlighter does not know at that time it is already in a string (the upper-level one: "`echo -n $xxx | tr -d \"'\\\"\"`.mp3". So it just handles the \" as an escape, and then sees the single quote as the start of a new string.

My bash accepts both the (a) and (b) construct:

$ x="`echo "'\\\""`"
$ echo $x
'"
$ x="`echo \"'\\\"\"`"
$ echo $x
'"

The bash.xml Katepart highlighter currently accepts only the (b) version.
The problem is here that bash.xml thinks the single quote in the inner string starts a new single quoted string (because we are in the backtick context).

A solution would be to handle \" inside a backtick concext the same way as ".

This is quite difficult to solve, but i will look into it.
Comment 6 Carsten Lohrke 2007-03-31 01:45:55 UTC
Current bash.xml accepts (a), but not (b). The single qoute is the start of string highlight. It's also not just the backtick block, but the backtick block inside of a double quote string block - and vice versa.

x="`echo \"'\\\"\"`"    is valid

x=`echo \"'\\\"\"`      isn't


The other way around:

x=`echo "'\""`          is valid

x="`echo "'\""`"        isn't

but both are highlighted as valid. Also bash.xml incorrectly highlights the following as valid

x=`echo "'\\\""`



So a whole chain of different string, backquote and substitution rules is needed, depending on the sourrounding context - unless there's a way to lookup the context stack.
Comment 7 Carsten Lohrke 2007-03-31 01:53:14 UTC
Created attachment 20134 [details]
bash.xml.diff

As it fits somewhat, I'm too lazy to open another bug and fixes two other
issues, here's a proposed change from Steve Long, which works nicely here.

https://bugs.gentoo.org/show_bug.cgi?id=172567
Comment 8 Matthew Woehlke 2007-04-02 20:02:43 UTC
Carsten Lohrke wrote:
> Current bash.xml accepts (a), but not (b). The single qoute is
> the start of string highlight. It's also not just the backtick
> block, but the backtick block inside of a double quote string
> block - and vice versa.
> 
> x="`echo \"'\\\"\"`"    is valid


My bash highlighter thinks this is invalid. So does my brain. However 
bash seems OK with it. Why, I wonder? Maybe I am not clear on the 
expansion rules in this instance.

> x=`echo \"'\\\"\"`      isn't


Ditto, except bash also (correctly IMO) doesn't like it.

> The other way around:
> 
> x=`echo "'\""`          is valid
> 
> x="`echo "'\""`"        isn't
> 
> but both are highlighted as valid.


That's odd, because all of the following are valid:

echo "'\""
echo `echo "'\""`
x="$(echo "'\"")" <-- this should be syntactically equivalent?!

...so this feels like a bug in bash.
Comment 9 Carsten Lohrke 2007-04-03 00:22:14 UTC
Matthew, it's not a bug in bash, it's a feature. $(foo) and `foo` are not 100% equivalent. Read the command substitution paragraph in the bash man page.
Comment 10 Matthew Woehlke 2007-04-03 01:41:46 UTC
Ok, Eric Blake explained this, basically a " inside ``'s ends the previous ", which seems a completely nonsensical behavior IMO. I don't see how the referenced patch is at all relative to this, however. As I see it there are two options:

1. Create a separate context for when we are in " and ` (that order) so that " is an error (I don't see how it could be other than an error, since there is an unmatched `)
2. Don't highlight inside ``'s in ""'s.

(2) is easy but clearly undesirable. (1) is more complicated.
Comment 11 steveL 2007-04-08 00:47:20 UTC
Well, the second line added by the patch is similar to Wilbert Berendsen's suggestion in comment 4, but only escapes quotes. His is much more comprehensive of course.
<RegExpr attribute="String Escape" context="#stay" String="\\'" />
on line 685 is still useful for escaping single quotes.

You're right however that neither deal with the backtick issue.
With: x="`echo \"'\\\"\"`" I see the first \" correctly highlighted as an escaped but not the other two.  I don't see anything in StringDQ or SubstBackq to explain that?
It'd be nice to get this all sorted out as well as Bug 142904 so that bash syntax highlighting is finally correct. Can I ask: are error conditions supposed to be separately highlighted; I don't see any such context? ATM I see them because following lines are say highlighted as string.

Thanks for the software, and to Wilbert for the bash highlighting :) I'll try and think more on what's needed for parameter expansion, which is such an important feature (and marked TODO). Springs to mind is handling a word and then detecting % # (or doubles) with perhaps a separate context for / substitution.
Comment 12 Matthew Woehlke 2007-04-10 16:14:15 UTC
I don't think there is an error context because I think this is the first instance where we *know* something is an error. Basically, the sequence "`" (with none of those escaped) is always illegal. For anything else, maybe you /meant/ to write what you did. :-)

Um... don't do parameter expansion, I already did that (no need to duplicate work, and mine is already pretty sophisticated). If I get a chance I'll take another look at integrating my changes, otherwise I would be happy to send you my copy of bash.xml.
Comment 13 steveL 2007-04-13 16:02:26 UTC
I think you're right then:
"Create a separate context for when we are in " and ` (that order) so that " is an error (I don't see how it could be other than an error, since there is an unmatched `)"
This new context needs to allow sinqle-quotes etc as per a standard DQ, but as you say error out on ", and drop back to DQ on `.
Sorry to repeat stuff I'm trying to get it clear:
x="`echo "'\""`" shows the need for the new "` context (err in bash, valid in kate) and the error context.
x="`echo \"'\\\"\"`" shows the need for the SQ not to start string in this new "` context. (valid in bash, breaks in kate.)

x=`echo \"'\\\"\"` correctly breaks since the SQ is not in "" and doesn't work in bash.

x=`echo "'\\\""` breaking in bash seems really odd, and personally I do see that as a bug.

Um, anyway, actually wanted to ask for that copy of your bash.xml pls? :)

Also, can we put in Wilbert's fix of adding this to VarBrace (~l 705):
<RegExpr attribute="Escape" context="#stay" String="\\[][;&quot;\\'$`{}()|&amp;&lt;&gt;* ]" />
- and this for StringSQ (~ l. 685)
<RegExpr attribute="String Escape" context="#stay" String="\\'" />

Integrating the parameter stuff with those 2 changes seems fairly simple. Then it's just the new context and Bug 140511 (case) - Bug 140513 - Bug 140514 (parentheses) and Bug 142904 (here-string) to go!
Comment 14 Matthew Woehlke 2007-04-13 17:29:25 UTC
Created attachment 20263 [details]
My current bash.xml

slong@rathaus.eclipse.co.uk wrote:
> I think you're right then:
> "Create a separate context for when we are in " and ` (that order) so that "
> is an error (I don't see how it could be other than an error, since there is
> an unmatched `)"

Agreed.

> Um, anyway, actually wanted to ask for that copy of your bash.xml pls? :)

Ok, since you asked in the bug, I'll post it here (in case anyone else wants to
look, also :-)).
WARNING: because mine is so different from the stock one, you'll want to check
also if mine is missing any updates.

> Also, can we put in Wilbert's fix of adding this to VarBrace (~l 705):
> <RegExpr attribute="Escape" context="#stay" \
>  String="\\[][;&quot;\\'$`{}()|&amp;&lt;&gt;* ]" />

Um, that isn't legal syntax, is it? Can you give an example of something this
would fix?

> - and this for StringSQ (~ l. 685)
> <RegExpr attribute="String Escape" context="#stay" String="\\'" />

No, we can't. That isn't a legal escape. :-)

> Integrating the parameter stuff with those 2 changes seems fairly simple.
> Then it's just the new context and Bug 140511 (case), Bug 140513,
> Bug 140514 (parentheses) and Bug 142904 (here-string) to go!

You'll probably want to check if I fixed any of those, too. :-)
Anyway I would be more than happy to help review any changes you want to make
(or offer assistance if you get stuck ;-)).
Comment 15 Elmar Stellnberger (AT/K) 2007-04-28 12:51:13 UTC
 KWrite misinterpretes escaped quotes inside variable expressions as well:

hugo='"quotedstr"rest'
hugo="${hugo#\"quotedstr\"}"
-> first \" not shielded

hugo='"quotedstr"rest'
hugo=${hugo#\"quotedstr\"}
-> OK

(result: hugo=rest)
Comment 16 Christoph Cullmann 2010-02-20 14:50:15 UTC
Moving to wish list status. All highlightings works with heuristics and will never be perfect. Perhaps somebody can provide a patch, therefor not closing it.
Comment 17 Christoph Cullmann 2012-11-03 16:58:15 UTC
Please provide a patch for the issue. We have more than 200 hls, we can't fix all such glitches ourself ;)
Comment 18 Andrew Crouthamel 2018-09-23 02:25:45 UTC
Dear Bug Submitter,

This bug has been in NEEDSINFO status with no change for at least 15 days. Please provide the requested information as soon as possible and set the bug status as REPORTED. Due to regular bug tracker maintenance, if the bug is still in NEEDSINFO status with no change in 30 days, the bug will be closed as RESOLVED > WORKSFORME due to lack of needed information.

For more information about our bug triaging procedures please read the wiki located here: https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging

If you have already provided the requested information, please set the bug status as REPORTED so that the KDE team knows that the bug is ready to be confirmed.

Thank you for helping us make KDE software even better for everyone!
Comment 19 Andrew Crouthamel 2018-10-27 04:14:28 UTC
Dear Bug Submitter,

This bug has been in NEEDSINFO status with no change for at least 30 days. The bug is now closed as RESOLVED > WORKSFORME due to lack of needed information.

For more information about our bug triaging procedures please read the wiki located here: https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging

Thank you for helping us make KDE software even better for everyone!