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

Questions about string literal highlighting and hover #1543

Open
michaelmesser opened this issue Jun 12, 2021 · 11 comments
Open

Questions about string literal highlighting and hover #1543

michaelmesser opened this issue Jun 12, 2021 · 11 comments

Comments

@michaelmesser
Copy link
Contributor

michaelmesser commented Jun 12, 2021

I am going to make string literal highlights not overlap the highlights in between\{ and }. Before I start I want to know if anyone wants other changes. This ended up being more complex that I anticipated and I might not get to it for some time.

Should string literals have their own color?
Should string literals stay as data?
Should they be highlighted based on fromString?
Should \{ and }be highlighted differently? Based on ++? Keyword?
Should " be highlighted as keyword?

What should hovering over a string literals show?
The type of fromString?
Should \{ and } show the type of ++?
What should hovering over " show?

@michaelmesser
Copy link
Contributor Author

michaelmesser commented Jun 14, 2021

I would prefer:
Highlight:

"Test \{  ?  } Test"
sssssskk     k sssss

where
s = String
k = Keyword

Hover:

"Test \{  ?  } Test"
ffffff++     +ffffff

where
f = fromString
+ = (++)

However since there is only one fromString getting it to show up twice could be difficult. Maybe only putting it on the leading string part would work. Also not sure if there is an easy way to make \{ keyword and have (++) as the name. Maybe it should be highlighted the same as (++). If anyone has a better proposal post it.

@andrevidela
Copy link
Collaborator

andrevidela commented Jun 14, 2021

Interpolated strings are not meant to overlap with fromString (last bit of section 2 #555) where does that come up?

An additional reason that is not mentioned there is that if you escape your string literals with a custom fromString then your starting or ending delimiter might be messed up by the escaping.

the delimiters should definitely be colored, not sure which coloring is more natural between keyword or function, but the goal of coloring it is to make it more obvious what is and what isn't interpolated when using them with raw strings

#"this is not \{interpolated} but this \#{ one } is"#

@michaelmesser
Copy link
Contributor Author

Interpolated strings are not meant to overlap with fromString (last bit of section 2 #555) where does that come up?

> `("test \{x}")

IApp (MkFC "(interactive)" (0, 2) (0, 13)) (IVar (MkFC "(interactive)" (0, 2) (0, 13)) (UN "fromString")) (IApp (MkFC "(interactive)" (0, 3) (0,
8)) (IApp (MkFC "(interactive)" (0, 10) (0, 11)) (IVar (MkFC "(interactive)" (0, 10) (0, 11)) (UN "++")) (IPrimVal (MkFC "(interactive)" (0, 3) (0,
8)) (Str "test "))) (IVar (MkFC "(interactive)" (0, 10) (0, 11)) (UN "x")))

@andrevidela
Copy link
Collaborator

andrevidela commented Jun 15, 2021

Yeah that's a mistake, whoopsies

@ohad
Copy link
Collaborator

ohad commented Jun 15, 2021

Since there's not much discussion, here are my opinions:

Should string literals have their own color?

Not until we have a proposal for a refined collection of decorations, and ideally a 2-tier hierarchy so that lean, small, clients can implement the bottom layer of the hierarchy without much work (like the current 5-constructor datatype), but the top layer can refine for the benefit of more sophisticated clients. (#1343 could be a good place to discuss it, or in a new proposal.)

Should string literals stay as data?

That's a decent option, and I'm comfortable with it for the time being, once the current bug where interpolated string is erroneously included in the string.

Should they be highlighted based on fromString?

Perhaps. I'm not sure how interpolated strings are desugared into fromString.
But if we want to highlight strings in a colour that's different to Data, I'd say that the colour should depend on the semantics.
Perhaps the string part should be highlighted according to the head symbol of its normal form:

  1. Typ if the fromString for it normalises to a term with a type constructor in head position.
  2. Data if the fromString for it normalises to a term with a data constructor in head position.
  3. Function in any other case.

Should { and }be highlighted differently? Based on ++? Keyword?

Based on ++, just like lists and snoclists:

  1. Typ if it desugars into a (++) that's disambiguated to a type-constructor
  2. Data if it desugars into a (++) that's disambiguated to a data-constructor
  3. Function if it desugars into a (++) that's disambiguated to a defined function
  4. Keyword if the desugared interpolate string doesn't include (++) for this delimiter (because it's at the edge of a string and so no (++) is inserted.

If two delimiters cause one (++), like in "\{"Hello"}\{"there"}", they should have the same, shared, highlighting, so:

"\{"Hello"}\{"there"}"
kkkdddddddfffdddddddkk

The answers to these questions combine for all kinds of edge cases, so I say we should resolve all of them before starting to implement this. For example, in case we decide to support a 'StringLiteral' semantic category in the top tier of decorations, then we need to highlight the whole string as below in the lower tier, and in the higher tier to highlight the string's delimiters with that decoration, and the string text itself with the 'StringLiteral' decoration (marked as l below):

higher tier: kkkdllllldfffdllllldfld
             "\{"Hello"}\{"there"}!"
lower tier:  kkkdddddddfffdddddddfdd

@michaelmesser
Copy link
Contributor Author

michaelmesser commented Jun 17, 2021

Perhaps the string part should be highlighted according to the head symbol of its normal form:

Typ if the fromString for it normalises to a term with a type constructor in head position.
Data if the fromString for it normalises to a term with a data constructor in head position.
Function in any other case.

I'm not quite sure how to implement that. I think I could make it the same color is the fromString but you are suggesting something more complex. I think I understand how to implement the rest.

@andrevidela
Copy link
Collaborator

fromString should again never appear in string interpolation, therefore only the last case matters

@ohad
Copy link
Collaborator

ohad commented Jun 17, 2021

I don't understand @andrevidela 's answer.

@ShinKage
Copy link
Contributor

If I understand @andrevidela correctly, there is inherent ambiguity on where fromString should be applied when elaborating interpolated strings (looking at the linked proposal). However, IMHO, there is no issue here, we just report whatever behaviour the compiler currently supports, as we already partially do.
Regarding the proposal, I like very much the two-tier approach proposed by @ohad, and I agree we must first decide what are the categories of the two tiers.

@michaelmesser
Copy link
Contributor Author

@ohad Are you saying "s should always be keyword? Or just before \{ and after }?

@ohad
Copy link
Collaborator

ohad commented Jun 19, 2021

I'm saying: if a " doesn't desugar into anything, then it should be decorated as a keyword. For example, I wrote:

             "\{"Hello"}\{"there"}!"
lower tier:  kkkdddddddfffdddddddfdd

because I think this dedsugars into:

original: "\{"Hello"}\{"there"    }!"
desugar :    "Hello" ++"there" ++ "!"
             ddddddd ffddddddd ff ddd

as you can see, some characters in the original string don't correspond to anything in the desugaring code. I proposed to highlight them as keywords. Another option is not to highlight them at all.

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

No branches or pull requests

5 participants