search

mathworks / MATLAB-Language-grammar

This repository contains a regular expression based language grammar for MATLAB to be used by GitHub Linguist for highlighting MATLAB code on GitHub
49 stars 17 forks source link

Fixes for import pattern #51

Closed watermarkhu closed 1 year ago

watermarkhu commented 1 year ago

The current regex for the meta.import.matlab scope has bugs with regard to import wildcards and trailing whitespaces. The following illustration from regex101 shows the whitespaces better.

image

For the wildcard itself there already exists a nested scope variable.language.wildcard.matlab:

https://github.com/mathworks/MATLAB-Language-grammar/blob/33f9678cf14662f44f2687e298826feb14015265/Matlab.tmbundle/Syntaxes/MATLAB.tmLanguage#L1783

Thus the fixes are needed in the main meta.import.matlab scope.

image

dklilley commented 1 year ago

Thank you for noticing this and making this change!

Can you add a test which verifies your change?

watermarkhu commented 1 year ago

Do you reckon this is a test that belong in the snap directory?

dklilley commented 1 year ago

I think it would make more sense to be a standalone test under the test/ directory. I believe the snapshot tests are meant to capture much more broad tokenization, rather than specific behaviors, to prevent a change from breaking some core functionality.

Instead, I think it would be better to create a focused test which covers the cases from your above regex101 screenshots.

I believe the number preceding the test (i.e. t##TestPoint) should correspond to the issue being fixed. If there is no issue for this bug, you can use the number of this pull request (51).

nothans commented 1 year ago

Thanks for the contribution. Can you contact me to get a CLA signed? Thank you!

nothans commented 1 year ago

The CLA has been signed. Feel free to evaluate the PR and determine if you are going to merge. Thanks!

dklilley commented 1 year ago

Thank you for making this fix @watermarkhu!