typescript.el icon indicating copy to clipboard operation
typescript.el copied to clipboard

Add support for expressions and nested strings in templates

Open lddubeau opened this issue 8 years ago • 22 comments

[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 avatar Aug 22 '17 11:08 lddubeau

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?

josteink avatar Aug 22 '17 11:08 josteink

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.

lddubeau avatar Aug 22 '17 12:08 lddubeau

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.

josteink avatar Aug 24 '17 06:08 josteink

Any news about this ?

danielpza avatar Feb 01 '19 20:02 danielpza

Any update on this? Its been years.

root0x avatar Apr 19 '22 19:04 root0x

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. :smile:

josteink avatar Apr 20 '22 11:04 josteink

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 avatar Apr 20 '22 11:04 theothornhill

@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 :)

theothornhill avatar Apr 20 '22 12:04 theothornhill

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 :grin:

josteink avatar Apr 20 '22 12:04 josteink

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 avatar Apr 20 '22 12:04 theothornhill

pushed

theothornhill avatar Apr 20 '22 12:04 theothornhill

I've opened this bug report but I'm fine with anybody taking it over. I've had a slew of life changing events in my life that make it so that I'm probably not going to return to making changes to this mode. I haven't done substantial software work in about 2 years, and I'm now focussing on other things, like writing, producing videos, and counseling people with life changing diseases or LGBTQ+ issues.

I'm not saying I'll never return to programming, but the chances are slim at this stage.

On Wed, Apr 20, 2022 at 8:39 AM Jostein Kjønigsen @.***> wrote:

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 😁

— Reply to this email directly, view it on GitHub https://github.com/emacs-typescript/typescript.el/issues/48#issuecomment-1103884369, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAO7LZIGRDL74GTCPQ54VFLVF73HJANCNFSM4DXZ7TMA . You are receiving this because you authored the thread.Message ID: @.***>

lddubeau avatar Apr 20 '22 12:04 lddubeau

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

theothornhill avatar Apr 20 '22 12:04 theothornhill

Sounds good @theothornhill!

And best of luck to you @lddubeau !

josteink avatar Apr 20 '22 13:04 josteink

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.

mattchrist avatar Apr 20 '22 14:04 mattchrist

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 :)

theothornhill avatar Apr 20 '22 15:04 theothornhill

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? :smile:

josteink avatar Apr 22 '22 13:04 josteink

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

theothornhill avatar Apr 22 '22 13:04 theothornhill

(though not finished yet) :)

theothornhill avatar Apr 22 '22 13:04 theothornhill

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

vbraun avatar Jul 27 '22 08:07 vbraun

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

josteink avatar Jul 30 '22 10:07 josteink

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 :)

Fuco1 avatar Aug 05 '22 20:08 Fuco1

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 😄

josteink avatar Oct 12 '23 07:10 josteink

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

kuba-orlik avatar Oct 16 '23 15:10 kuba-orlik