Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Array closing parenthesis highlighted incorrectly #60

Closed
vojtagrec opened this issue Feb 6, 2015 · 11 comments
Closed

Array closing parenthesis highlighted incorrectly #60

vojtagrec opened this issue Feb 6, 2015 · 11 comments

Comments

@vojtagrec
Copy link

The closing parenthesis in an array() statement (= the one having class="punctuation definition array end php") seems to be selected incorrectly – simply the first ) character is selected, which is not correct if there is a function/method called inside the array() definition. See the screenshot:

image

It is using Seti syntax theme which assigns the array() parentheses a different colour (light blue in this case).

Atom 0.177.0 on Mac OS X 10.10.2

@vojtagrec vojtagrec changed the title Array closing bracket highlighted incorrectly Array closing parenthesis highlighted incorrectly Feb 6, 2015
@winstliu winstliu added the bug label Feb 6, 2015
@zr9
Copy link
Contributor

zr9 commented Jan 19, 2016

I'm can add fix for single line, but multiline now not possible due to grammar limits looks please issue #87 for details. For single line is possible. @50Wliu That way it be ok?

@winstliu
Copy link
Contributor

@zr9 could you try changing the array regex to a begin/end match instead of a match? begin/end can match multiple lines.

@Ingramz
Copy link
Contributor

Ingramz commented Jan 19, 2016

@50Wliu end will stop at the first occurrence of the pattern you are looking for.

If we take the image above as an example - if you were looking for ), it would stop on the second line at Is Artist?') and rest of it will be captured by some other rule. By looking at the coloring of the parentheses, I think this is exactly what it is doing already.

Since you can only match a single line at a time, then the best case would be that we'd be stopping at the last occurrence of closing parentheses of that line. That wouldn't solve the issue with multiple lines.

To balance parentheses, we'd need a stack in order to keep track of the state. That we simply cannot do with these grammars.

@winstliu
Copy link
Contributor

@Ingramz no, you can capture parentheses within the match, eg:

'begin': 'array\\('
'end': '\\)'
'patterns': [
  {
    'begin': '\\('
    'end': '\\)'
    'patterns': [
      {
        'include': '$self'
      }
    ]
  }
]

This match will only end when there's unbalanced parentheses or when it's supposed to (I think).

language-javascript, c, and I believe some others match brackets this way.

EDIT: #self -> $self, thanks @Ingramz

@zr9
Copy link
Contributor

zr9 commented Jan 19, 2016

@50Wliu Yup could be, need to test, because it hard to say how grammar work inside, not a much docs available, is grammar in that case works like recursive regex then it will work, otherwise there is possible probably another way with capturing array content in begin(with recursion as well), i'm try and get back to you in a few hours

@Ingramz
Copy link
Contributor

Ingramz commented Jan 19, 2016

@50Wliu took a look at one of my older grammars and found out that I've even used that kind of construct. Thanks for the reminder, this definitely gave me some ideas.

But in the context of PHP, that's quite a bit of work to get all the constructs matching that way correctly.

@zr9
Copy link
Contributor

zr9 commented Jan 19, 2016

@Ingramz Could you give me some more info on that construction, is it supposed to work as recursion? Because now i'm trying to use it, and so far had 0 luck in that

@Ingramz
Copy link
Contributor

Ingramz commented Jan 19, 2016

@zr9 yes, it should be recursive. Try $self instead of #self.

@zr9
Copy link
Contributor

zr9 commented Jan 19, 2016

@50Wliu @Ingramz Okay bad news and good news, bad first.

  1. The rule above is not exactly recursive it kind of include scope which in patterns
  2. There is some bug in grammar parser or in itself grammar(not yet know)

Good news :)

array_issue

That rule inside patterns is can skip some scopes inside of itself it they kind of busy(based on tests, any docs on that are welcome). So it possible, but there problems, i'm still not getting logic behind grammar, in which cases it targeting and in which not. For bug i'm give more info once i'm get it at least little clear

@zr9
Copy link
Contributor

zr9 commented Jan 19, 2016

Yup grammar bug, not logic bug, and for rule 1, it works only when scope is get busy with included pattern, then it skip that one since it busy and match next one, that info is from observations and test so can be not 100% true. Itself whole thing is possible but require issue #47 to be done in prior, and probably serious rules update, since parentheses need not only in global space but as well in many other scopes

@winstliu
Copy link
Contributor

winstliu commented Jun 9, 2017

Closing in favor of #47.

@winstliu winstliu closed this as completed Jun 9, 2017
@winstliu winstliu added duplicate and removed bug labels Jun 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants