Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grammar Improvements #18

Closed
wants to merge 3 commits into from
Closed

Conversation

sanssecours
Copy link
Member

Hi,

this pull request includes two fixes for the bundle grammar. The screenshot below shows the effect of the changes using the Rainbow Dash theme:

haskell - highlighting

Hope the grammar updates are helpful. If I should change anything, then please just comment below.

Kind regards,
René

Before this change TextMate would use the wrong scope
(`keyword.other.double-colon.haskell`) for the second equal sign in the
following Haskell code:

```hs
instance Eq Time where
--    ↓ Wrong scope `keyword.other.double-colon.haskell`
    (==) :: Time -> Time -> Bool
```

.
Before this change TextMate would incorrectly highlight parts of
function signatures that continue over multiple lines. For example the
third `Integer` in the following code:

```hs
add :: Integer -> Integer ->
       Integer -- ← Wrong scope `constant.other.haskell`
```

would use the scope `constant.other.haskell` instead of the correct
`support.type.prelude.haskell`.
@JustusAdam
Copy link

I copied your changes here verbatim into JustusAdam/language-haskell@b195289 and there arises the issue that function names are not recognized as such anymore. Can you explain why that is?

The markup generally works (for multiline type signatures too) but the function names are not marked as functions anymore

screen shot 2016-12-20 at 2 08 54 pm

@sanssecours
Copy link
Member Author

Hi Justus,

thank you for your feedback.

I copied your changes here verbatim into JustusAdam/language-haskell@b195289 and there arises the issue that function names are not recognized as such anymore. Can you explain why that is?

I assume you refer to the name of the function in the function signature? E.g. add in the following code:

add :: Integer -> Integer ->
       Integer

or the first print_ in your screenshot? Could you please add a minimal example, where you show how the highlighting worked before, and how the changes in this pull request broke the syntax highlighting?

@JustusAdam
Copy link

Yes. I made another two screenshots for you, both using the multi line type signatures as well.

Before
screen shot 2016-12-21 at 8 23 00 am

After
screen shot 2016-12-21 at 8 24 21 am

@sanssecours
Copy link
Member Author

sanssecours commented Dec 21, 2016

Thank you for adding the screenshots. It looks like the cause of this behaviour could be a bug in the parsing code of Visual Studio Code (VSC). If I change the while-pattern from ^\1\s+ to ^\s*, then the syntax highlighting works correctly for a single multiline signature:

Visual Studio Code – Modified While

. If I change the while-pattern back, then print_ is not highlighted any more:

Visual Studio Code - Unmodified While

. In my opinion the while-pattern should not change the behaviour of the highlighting of the previous line at all. Since the changes work in TextMate:

TextMate - Unmodified While

, I think it is best you open an issue at the bug tracker of Visual Studio Code and ask them why VSC behaves differently.

@JustusAdam
Copy link

JustusAdam commented Dec 21, 2016

Thanks a bunch. I'll open an issue there. Is there any other way we can phrase the pattern to make it work? Because the alternate pattern breaks all the other markup.

@JustusAdam
Copy link

Actually, I have a suggestion that seems to fix it:

I changed the while pattern to

<key>end</key>
<string>^[^\s]</string>

What do you think? will that work?

@sanssecours
Copy link
Member Author

Is there any other way we can phrase the pattern to make it work?

I can not think of an easy way to fix this. In my opinion the pattern ^\1\s+ already describes the intention of including all lines in the signature that are indented more than the first line of the signature pretty well. If you want to try it yourself, then here are some resource that are usually quite helpful:

A Description of Regular Expressions and Regex Engines
The Section “Language Grammars” of the TextMate Manual
Writing a TextMate Grammar: Some Lessons Learned

.

Because the alternate pattern breaks all the other markup.

Yeah, I know. If the simpler pattern would work I would have used it myself 😊.

@JustusAdam
Copy link

I just tried it out, and I don't think you need the complex pattern anyways.

addReaction :: ActionData d 
    -> BotReacting a d () 
  -> ScriptDefinition a ()

I just compiled a file with this type signature in it and it worked fine. It seems the continued type signature doesn't have to be indented as far as the first, or preceding line, so long as it is indented at all.
That would mean my end pattern is actually correct.

@JustusAdam
Copy link

Never mind I see the issue. I was looking at the wrong indentation.

@sanssecours
Copy link
Member Author

I changed the while pattern to

<key>end</key>
<string>^[^\s]</string>

What do you think? will that work?

This pattern will not work if you indent functions:

syntax highlighting

.

@JustusAdam
Copy link

JustusAdam commented Dec 21, 2016

Yeah, I ran into that immediately. But I have good news, it seems the issue is the while template. This one works fine:

<key>end</key>
<string>^(\1)?(?=[^\s])</string>

@JustusAdam
Copy link

Another thing is that multiline type signatures do not yet work when they start on the following line.

Aka this works

screen shot 2016-12-21 at 2 36 50 pm

And this doesn't

screen shot 2016-12-21 at 2 37 01 pm

Is there any way we can make that work as well?

@sanssecours
Copy link
Member Author

Is there any way we can make that work as well?

I do not think so, how would you distinguish between

  • setSession as part of the function signature and
  • setSession as part of the definition of the function

if there are no additional signs (e.g. = or ::) in the same line? Or in other terms, is the setSession below part of the function signature or part of the function definition:

setSession

?

@JustusAdam
Copy link

Yeah I guess so. Thats the problem with parsers that only look at one line at a time.

You'd probably need some general rule for indented blocks of lines and then distinguish different cases (expression, signature, definition, etc) for the entire block

@infininight
Copy link
Member

Pulled in, thanks @sanssecours!

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

Successfully merging this pull request may close these issues.

3 participants