Menu

#1947 TCL Double quote not recognized after a comma

Bug
closed-fixed
nobody
4
2019-03-07
2017-05-24
ericc
No

Hello

I'm using Geany, a nice editor which, apparently, use Scintilla in the background
I'm coding in TCL mainly, and today I faced a annoying behavior
I used the following syntax :
set n $showArray($i,"neighbor")
This code is perfectly valid and recognized by TCL
However, the first double quote after the comma seems to not being recognized by Geany/Scintilla and thus all my later code is highlighted as a string !!
which is really annoying

If I put a space after the comma, then the first quote is seen and the code is correctly highlighted ... but not valid anymore

I tried with different editors apparently also based on Scintilla, including the last version of wScite and I saw the same behavior on all of them

What can I do to solve the problem ?

Thanks in advance for your help

ericc

Discussion

  • Thorsten Kani

    Thorsten Kani - 2017-05-24

    Hi ericc, like you, i was curious about changes to scintillas lexers. In my case, i recently discovered that the makefile lexer didnt colorize Variables and automatic variables in gnu makefiles.
    today, i decided to look at that issue and was able to implement the desired behaviour. maybe the code can help you to fix your topic with tcl. link

     
  • Neil Hodgson

    Neil Hodgson - 2017-05-24
    • labels: TCL --> TCL, scintilla, lexer
    • status: open --> open-accepted
    • Priority: 5 --> 4
     
  • Neil Hodgson

    Neil Hodgson - 2017-05-24

    The lexer appeas to be doing this deliberately with the ',' being treated specially when inside a bracketed $ expression.

    Any change would require modifying the lexer at scintilla/lexers/LexTCL.cxx.

     
  • Thorsten Kani

    Thorsten Kani - 2017-05-27

    Seems like a bug.in statemachine code when handling unclosed doublequotes. tried $($,"" ")

     
  • Zufu Liu

    Zufu Liu - 2019-01-24

    This can be fixed by replacing goto with continue.
    ; is highlighted as operator in this patch, also I think highlight substitution inside string will be useful by adding following code (included in tcl2.diff) into case SCE_TCL_IN_QUOTE:

    +           } else if (sc.ch == '$' && IsAWordStart(sc.chNext)) {
    +               sc.SetState(SCE_TCL_SUBSTITUTION);
    +               sc.Forward();
    +               while (sc.More() && IsAWordChar(sc.ch)) {
    +                   sc.Forward();
    +               }
    +               sc.SetState(SCE_TCL_IN_QUOTE);
    +               continue;
    
     
    • Neil Hodgson

      Neil Hodgson - 2019-01-27

      The tcl.diff patch has changed multiple things - it has reorganised the code to prefer continue over goto next; it has changed the behaviour of , inside braces; indirectly changed the behaviour of ( in a substitution; changed ; from SCE_TCL_DEFAULT to SCE_TCL_OPERATOR; and potentially changed other behaviour that I haven't yet understood. If this causes regressions, it will be more difficult than necessary to understand the cause of the regression and so fix it.

      A change set should fix a single bug at a time. That ensures that maintainers can check that change against a range of examples to see if it is incorrect and more easily understand how to fix it if needed. If the code needs extensive reorganising in order to apply a fix, then that reorganisation should be a separate change set that behaves identically to the previous state.

      The purpose of the reorganisation appears to be to avoid the automatic sc.Forward() in the for loop and thus the need for the next label and goto next which is currently used to avoid the sc.Forward().

      A short, isolated change set to avoid the sc.Forward() for a ',' inside a bracketed substitution appears to be:

      diff -r a82b87a88556 lexers/LexTCL.cxx
      --- a/lexers/LexTCL.cxx Thu Jan 24 10:41:41 2019 +1100
      +++ b/lexers/LexTCL.cxx Sun Jan 27 11:14:26 2019 +1100
      @@ -128,8 +128,10 @@
                      continue;
                  case ',':
                      sc.SetState(SCE_TCL_OPERATOR);
      -               if (subParen)
      +               if (subParen) {
                          sc.ForwardSetState(SCE_TCL_SUBSTITUTION);
      +                   goto next;  // Already forwarded so avoid loop's Forward()
      +               }
                      continue;
                  default :
                      // maybe spaces should be allowed ???
      

      The above change set is more difficult to understand than the corresponding chunk of tcl.diff so committing a sequence of isolated change sets: reorganisation then bug fix, would be preferrable.

      Changing ';' to operator was not part of this bug report and produces many differences in output which may affect downstream projects. It should have its own bug report or feature request and its own change set.

       
  • Zufu Liu

    Zufu Liu - 2019-01-27

    Except changing ';' to operator, other goto next to continue changes I think is technically equivalent.
    The for loop is changed to while loop, sc.Forward() in for () is moved to the end of the while loop.

     
  • Neil Hodgson

    Neil Hodgson - 2019-01-29
    • status: open-accepted --> open-fixed
     
  • Neil Hodgson

    Neil Hodgson - 2019-01-29

    Committed short fix above as [f75e25] along with a unit test case.

     

    Related

    Commit: [f75e25]

  • Neil Hodgson

    Neil Hodgson - 2019-03-07
    • status: open-fixed --> closed-fixed
     

Log in to post a comment.