Skip to content

Add support for expressions and nested strings in templates #48

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

Closed
lddubeau opened this issue Aug 22, 2017 · 24 comments
Closed

Add support for expressions and nested strings in templates #48

lddubeau opened this issue Aug 22, 2017 · 24 comments

Comments

@lddubeau
Copy link
Collaborator

[Opening an issue for a problem discovered while discussing #20 .]

Template strings allow expressions in string and strings to nest. For instance:

`a${1+2}b`

equals "a3b". And since expressions can be strings, then this is valid:

`a${`b`}c`

and equals "abc"

Problematic areas:

  1. The highlighter fails to handle anything in ${} as an expression. (Trying the same in plain JS code: js-mode is just as broken as typescript-mode. js2-mode, however, handles it fine.)

  2. typescript--re-search-backward-inner completely ignores expressions in strings or the fact that strings can nest. Same for typescript--re-search-forward-inner. These are used for movement across syntactic features and for indentation.

  3. syntax-ppss, which is also used for movement across syntactic features and for indentation, ignores string nesting. js2-mode appears to have solved the issue though because I get meaningful results from checking the values of syntax-ppss in nested strings in a JS file with js2-mode.

There may be yet other problematic areas I've not identified.

@lddubeau lddubeau added the bug label Aug 22, 2017
@josteink
Copy link
Member

josteink commented Aug 22, 2017

The highlighter fails to handle anything in ${} as an expression

I'm going to just assume that however we decide to handle this, it ultimately comes down to syntax-tables and other relatively low-level cruft.

To my knowledge syntax-tables have no understanding of interrupting string-sections with intermediate code-blocks by a string-delimiter-escape-character, before the subsequent string-section continues until the original string-delimiter is met (and whatever evil cyclic conditions one can manufacture using this technique).

We can try hack our way around this, but if this functionality is needed in js-mode too anyway, I think it would be best (yes, I know my previous statements about these matters), to first investigate with emacs-devel if they:

  1. are aware of the problem/use-case
  2. are planning any changes to syntax-table related functionality to accommodate syntax like this.

There's no need to reinvent wheels emacs-devel is making for us (if they are making it for us).

Agreed? Disagreed?

@lddubeau
Copy link
Collaborator Author

I agree. It may turn out that typescript-mode will remain its own independent thing rather than aggressively derive from js-mode but even if that's the case, we can benefit from any fix made to js-mode.

I'll note again that js2-mode appears to have fixes for the highlighting and syntax-ppss so we can look at that if js-mode remains broken.

@josteink
Copy link
Member

I've written a fairly detailed email about the issue to emacs-devel and put you on CC.

Feel free to chime in on the issue and thread if you feel like I missed something important.

@danielpza
Copy link

Any news about this ?

@root0x
Copy link

root0x commented Apr 19, 2022

Any update on this? Its been years.

@josteink
Copy link
Member

I think the only realistic way this is getting solved is if somebody is willing to do what @theothornhill did for csharp-mode: start a "from scratch" rewrite based on the tree-sitter package and library.

IMO that rewrite worked out really well. So well in fact I tried to do the same for typescript-mode, but I quickly backed off once I realized how far I was from being able to complete it ;)

If somebody else however wants to take a stab at that, I would fully support such an initiative. 😄

@theothornhill
Copy link
Collaborator

Hi! I'm actually looking to do this, since I'm a little bothered by the indentation troubles with typescript. In particular inside jsx. I've been holding off a little because I was waiting for the native implementation by Yuan Fu to land in emacs proper. I could do the same for typescript mode if you want, but I'm a little unsure if it is best to wait for emacs proper. I saw you added me to this repo, so I can start working on a branch anyways if you'd like. I write a lot of ts these days, so my motivation is improving, hehe.

I think it should be pretty fast to do - the most problematic things would probably be setting the indentation properly, but most people use prettier anyways these days don't they?

Thinking about it I think most of the work should be in the indentation engine.

@theothornhill
Copy link
Collaborator

@josteink how about I take over your branch and turn the pr into a working draft for now? Then we'll see how far I get :)

@josteink
Copy link
Member

Of all bad luck, I think I only had that as a local branch... On a laptop I had Ubuntu on, but literally yesterday reformatted and installed Debian on instead. Ooops 😁

@theothornhill
Copy link
Collaborator

No I found it. No problem! I've started the work. Some indentation is working already, so I think this will be quick. How about we cooperate on this? I'll push what I have and we can just use it and extend it? Right now all you need to do is to C-M-x the tree-sitter-indent-typescript-tree-sitter-scopes inside of typescript-tree-sitter.el and revert the buffer, and the new indentation can be used. tree-sitter-debug-mode is a must to make this work. So when you are in a meeting you can do something useful :) I'll just push what I have now and start using it myself. Sounds good?

@theothornhill
Copy link
Collaborator

pushed

@lddubeau
Copy link
Collaborator Author

lddubeau commented Apr 20, 2022 via email

@theothornhill
Copy link
Collaborator

Good luck on your endeavours - the code'll be here should you decide to return :)

@josteink
Copy link
Member

Sounds good @theothornhill!

And best of luck to you @lddubeau !

@mattchrist
Copy link

It may be possible to leverage the semantic tokens feature from the language server to accomplish this as well. It seems supported by lsp-mode.

@theothornhill
Copy link
Collaborator

I'd advise against that. Imo it is not wise to offload more things to json. Also only lspmode supports that and not all servers do. Lastly, one can always enable it on top of tree sitter.

Just my 2 cents :)

@josteink
Copy link
Member

Like theo has already said in the related PR, it doesn't even indent yet... But for those looking for template-strings and similar stuff...

image

Not bad for a 1 day job, eh? 😄

@theothornhill
Copy link
Collaborator

We do indeed indent now :) and also support the new emacs proper

@theothornhill
Copy link
Collaborator

(though not finished yet) :)

@vbraun
Copy link

vbraun commented Jul 27, 2022

Possibly related corner case: Non-backtick quote chars (i.e. ' and ") inside template strings apparently make the syntax highlighting think that a string starts there:
Screenshot from 2022-07-27 10-36-34

@josteink
Copy link
Member

This is not a problem in the new tree-sitter based mode (tested with native emacs tree-sitter support from the emacs feature/tree-sitter branch):

image

@Fuco1
Copy link
Contributor

Fuco1 commented Aug 5, 2022

I've implemented a matcher for the ${} expression inside backticks

(defun typescript--match-subst-in-quotes (limit)
  "Match dollar substitutions inside backticks."
  (catch 'done
    (while (re-search-forward
            ;; `rx' is cool, mkay.
            (rx (or line-start (not (any "\\")))
                (group
                 "$"
                 (and "{" (+? nonl) "}")))
            limit t)
      (-when-let (string-syntax (nth 3 (syntax-ppss)))
        (when (= string-syntax 96)
          (throw 'done (point)))))))

somewhere in the keywords add

(typescript--match-subst-in-quotes
     (1 'default t))

I will add tests and make a PR these days but I'm traveling ATM. Feel free to try this out though :)

@josteink
Copy link
Member

josteink commented Oct 12, 2023

This issue is old and has seen no activity in a year+.

Closing this issue. Typescript support is now included in Emacs core, so if you still have problems, open a new issue on the GNU Emacs bug-tracker 😄

@kuba-orlik
Copy link

This is resolved by using typescript-ts-mode (the tree-sitter variant)

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

9 participants