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

Catch parameter not correctly highlighted when declared in new line or with a comment in between #219

Closed
Vigilans opened this issue Dec 5, 2019 · 4 comments · Fixed by #220
Labels

Comments

@Vigilans
Copy link
Contributor

Vigilans commented Dec 5, 2019

Description

Catch parameter name is highlighted as type when declared in new line or with a comment in between.

Steps to Reproduce

Paste the following code to editor:

public static void main(String[] args) {
    try {
        // something
    } catch // this is a comment
    (NoSuchMethodException |
        SecurityException
      | InstantiationException | llegalAccessException |
      IllegalArgumentException | invocationTargetException 
      e
    ) {
        e.printStackTrace();
    }
}

public static void main(String[] args) {
    try {
        // something
    } catch // this is a comment
    (NoSuchMethodException |
        SecurityException
      | InstantiationException | llegalAccessException |
      IllegalArgumentException | invocationTargetException /* comment */ e
    ) {
        e.printStackTrace();
    }
}

Expected behavior: e is highlighted as variable.parameter.java

Actual behavior:

e is highlighted as storage.type.java:
image

Reproduces how often: Always

@lkashef
Copy link

lkashef commented Dec 6, 2019

Hello @Vigilans, thanks for the report, I have simplified the examples, let me know if you have any comments.

Closing bracket in on a new line.

try {} catch (ex e
){}

Comment between the type and variable name.

try {} catch (ex /**/ e){}

@sadikovi
Copy link
Contributor

sadikovi commented Dec 7, 2019

It is basically two scenarios:

@sadikovi sadikovi added bug and removed bug labels Dec 7, 2019
@Vigilans
Copy link
Contributor Author

Vigilans commented Dec 7, 2019

@lkashef

try {} catch (ex e
){}

should be

try {} catch (ex
e){}

sadikovi pushed a commit that referenced this issue Dec 8, 2019
### Description of the Change

Uses a two-scope begin/end model to correctly highlight catch parameter.

It wraps `Exception ... |` or `Exception ... )` as a scope, allowing parameter to only sit in this scope, to resolve the parsing sequence issue (first exception type, then parameter identifier).

### Alternate Designs

#### A more complex two-scope design:
* scope 1: `(...Exception`
* scope 2: `|...Exception`
* scope 3: `parameter...)`

Problem: cannot correctly highlight when there's a storage-modifer (like final)

#### Recursive design:
```yaml
catch-parameters:
  begin: /(?<=\()|(\|)/
  beginCapture.0.name: 'catch.separator'
  end: /(?=\))/
  patterns:
    - include: storage-modifier
    - include: exception-type
    - include: catch-parameters # recursive
    - include: paramter-identifier
```

It makes use of the fact that `catch (ex1 | ex2 | ex3 param)` is in terms of structure the same as `catch (ex1 (ex2 (ex3 param)` (every time a `|` is met, the remaining part is actually a new catch-parameter pattern) 

Problem: still cannot elegantly handle the base case: `catch (final ex3 param)`. 

### Benefits

Can correctly highlight catch parameter when it is placed in new line or with a comment in between.

### Possible Drawbacks

Don't see any afak.

### Applicable Issues

Fix #219
@lee-dohm
Copy link
Contributor

Thanks @sadikovi! 🙇

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants