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

(elixir) interpolation, sigils and / functions #730

Closed
3 tasks done
eksperimental opened this issue Feb 3, 2015 · 21 comments · Fixed by #2406
Closed
3 tasks done

(elixir) interpolation, sigils and / functions #730

eksperimental opened this issue Feb 3, 2015 · 21 comments · Fixed by #2406
Assignees
Labels
enhancement An enhancement or new feature help welcome Could use help from community

Comments

@eksperimental
Copy link
Contributor

eksperimental commented Feb 3, 2015

Update 11/2019: As far as I know #2257 leaves only item 3 open.


We are adding hightlightjs to https://github.com/elixir-lang/ex_doc/
and when I ran it against the test file for pygments.
In addition to #707, I have run into three new issues:

  1. Interpolation: It cannot deal properly with interpolation.

Every single line bellow, has an issues, it takes it as a comment whatever comes after the #

~R'this + i\s "a" regex too'
~w(hello #{ ["has" <> "123", '\c\d', "\123 interpol" | []] } world)s
~W(hello #{no "123" \c\d \123 interpol} world)s
~s{Escapes terminators \{ and \}, but no {balancing} # outside of sigil here }
~S"No escapes \s\t\n and no #{interpolation}"
  1. Sigils : It cannot deal with these ~S sigil. ~s is fine though.
defmodule Long.Module.Name do
  @doc ~S'''
  No #{interpolation} of any kind.
  \000 \x{ff}

  \n #{\x{ff}}
  '''
  def func(a, b \\ []), do: :ok

  @doc ~S"""
  No #{interpolation} of any kind.
  \000 \x{ff}

  \n #{\x{ff}}
  """
  def func(a, b \\ []), do: :ok
end
  1. Function with a slash ( "Kernel./" ): this one takes /: as the beggining of a regular expression
import Kernel, except: [spawn: 1, +: 2, /: 2, Unless: 2]

You can see this live,

@Sannis Sannis added the bug label Feb 4, 2015
@Sannis Sannis self-assigned this Jul 15, 2015
@Sannis Sannis added this to the 8.8 milestone Jul 27, 2015
@Sannis Sannis changed the title 3 more issues with Elixir 3 more issues with Elixir: interpolation, sigils and imports with slashes Jul 29, 2015
@Sannis Sannis changed the title 3 more issues with Elixir: interpolation, sigils and imports with slashes 3 more issues with Elixir: interpolation, sigils and / functions Jul 29, 2015
@Sannis Sannis removed this from the 8.8 milestone Sep 3, 2015
@Sannis Sannis added this to the 8.9 milestone Sep 5, 2015
@Sannis
Copy link
Collaborator

Sannis commented Sep 7, 2015

@eksperimental to fix this I'm going to remove constant mode from Elixir definition. In fact it works bad here and in constructions like IO.puts "string" and also we will remove such extra modes in 9.0.

Sannis added a commit to Sannis/highlight.js that referenced this issue Sep 7, 2015
@eksperimental
Copy link
Contributor Author

@Sannis thanks a lot. please do as you consider convenient. We will adapt our CSS files accordingly.

Sannis added a commit to Sannis/highlight.js that referenced this issue Sep 7, 2015
@Sannis
Copy link
Collaborator

Sannis commented Sep 7, 2015

@eksperimental, am I right that sigils can be also included into string interpolations? It looks like this will be too CPU heavy to do so :-(

Sannis added a commit to Sannis/highlight.js that referenced this issue Sep 8, 2015
Sannis added a commit to Sannis/highlight.js that referenced this issue Sep 8, 2015
Relates to p.2 from highlightjs#730

Unfortunately, we do not distinguish s and S sigils with different interpolation rules
@Sannis
Copy link
Collaborator

Sannis commented Sep 8, 2015

@eksperimental, I fixed some cases and added more regression tests.
Unfortunately, we do not distinguish s and S sigils with different interpolation rules for now.
Also adding more sigils formats results in many variants in the modes definitions and increase highlighting and autohighlight time.

@eksperimental
Copy link
Contributor Author

that's great @Sannis
So one question. we don't support interpolation inside the ~S sigil, right?
if so, what happens when it's matched, does it break the code? or just ifgnores is and highlights it as regular text?
thank you for looking after this issue.
Elixir 1.1 is coming out soon, and a new version of ExDoc which uses this, so even if the next version of hightlight.js in not out, we will try to ship it outselves.

@Sannis
Copy link
Collaborator

Sannis commented Sep 10, 2015

I'll post highlighting example to the pull request thread today.

We currently do releases each 6 weeks, so next one will be only in October.

@Sannis Sannis added enhancement An enhancement or new feature and removed bug labels Sep 10, 2015
@isagalaev
Copy link
Member

Elixir 1.1 is coming out soon, and a new version of ExDoc which uses this, so even if the next version of hightlight.js in not out, we will try to ship it outselves.

Here's the building docs, just in case: http://highlightjs.readthedocs.org/en/latest/building-testing.html

@eksperimental
Copy link
Contributor Author

@isagalaev excellent! thanks a lot for all the help

@Sannis
Copy link
Collaborator

Sannis commented Sep 11, 2015

@eksperimental can you please post an example highlighting for ~s{Escapes terminators \{ and \}, but no {balancing} # outside of sigil here } that you expect? Am I right that whole this fragment is sigil?

@eksperimental
Copy link
Contributor Author

@Sannis nope, it is not, starting from # is a comment

This is how it gets interpreted by Elixir interactive shell

iex(1)> ~s{Escapes terminators \{ and \}, but no {balancing} # outside of sigil here }
"Escapes terminators { and }, but no {balancing"

so the { in {balancing is not balanced

@eksperimental
Copy link
Contributor Author

hi @Sannis is there anything else that you need from me? Or anything we can do to help you sort this out?

@isagalaev isagalaev modified the milestones: 8.10, 9.0 Oct 27, 2015
@umamaistempo
Copy link

About the issue 3, the same thing happens when using simplified anonymous function. I (wrongly) created an issue about this on Ex_doc repo: elixir-lang/ex_doc#486

(&fun!/1 is interpreted as a regex)

@isagalaev
Copy link
Member

Won't make it to 9.0…

@isagalaev isagalaev removed this from the 9.0 milestone Dec 2, 2015
@joshgoebel joshgoebel changed the title 3 more issues with Elixir: interpolation, sigils and / functions (elixir) interpolation, sigils and / functions Oct 13, 2019
@joshgoebel joshgoebel added the help welcome Could use help from community label Nov 5, 2019
joshgoebel pushed a commit to joshgoebel/highlight.js that referenced this issue Nov 5, 2019
Relates to p.2 from highlightjs#730

Unfortunately, we do not distinguish s and S sigils with different interpolation rules
@joshgoebel
Copy link
Member

@eksperimental Any chance you can test PR #2257? I think it fixes points 1 and 2.

@joshgoebel joshgoebel added this to Has open PR in Highlight.js Nov 5, 2019
@joshgoebel
Copy link
Member

@eksperimental Any chance you could have a look at #2257 and see what you think?

@joshgoebel
Copy link
Member

Oh sorry I already pinged you 3 days ago, lol... I guess think of it was a reminder. :)

@eksperimental
Copy link
Contributor Author

I there a live version where I can test this?

@joshgoebel
Copy link
Member

No, but I could probably build you a demo/devtool zip if it would truly be helpful.

@eksperimental
Copy link
Contributor Author

No, but I could probably build you a demo/devtool zip if it would truly be helpful.

if you could. I'm out of touch with this lib (and the whole JS ecosystem), and it's been 4 years since I opened up this issue. I 'm doing it out of appreciation becuase someone took the time to take care of this after all these years.
If you think it would be helpful for your PR for me to review it, please build it so. thanks

@joshgoebel
Copy link
Member

@eksperimental Sounds like you're busy - and I haven't gotten around to this either. I'm just going to run with this as it. It passes all the original examples provided. If there are additional edge cases we can just let people file them as they see them.

Thanks for the offer to help though!

@eksperimental
Copy link
Contributor Author

Cool. Thank you!

@joshgoebel joshgoebel removed this from Has open PR in Highlight.js Nov 13, 2019
joshgoebel added a commit to joshgoebel/highlight.js that referenced this issue Feb 17, 2020
- Do not mistakenly recognize `/:` as the beginning of a regex,
  which causes highlighting to abort.
- `/:`, `+:`, are still not considered symbols

This solves the problem by making the `/: \d` an exception case that is
never treated as a regex.  For most cases, most of the time this should
be a win.

Resolves highlightjs#730.
@joshgoebel joshgoebel added this to the 10.0 milestone Feb 17, 2020
@joshgoebel joshgoebel self-assigned this Feb 17, 2020
joshgoebel added a commit that referenced this issue Feb 17, 2020
- Do not mistake code like `import Kernel, except: [ /: 2 ]` as the beginning 
  of a regex, which causes highlighting to break.
- `/:`, `+:`, are still not considered symbols

This solves the problem by making the pattern `/: \d` an exception case that is
never treated as a regex.  For most cases, most of the time this should
be a win.

Resolves #730.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature help welcome Could use help from community
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants