ipatalas / vscode-postfix-ts

Postfix notation for TypeScript/Javascript - extension for VS Code
MIT License
158 stars 43 forks source link

Print render text instead of raw body #61

Closed zardoy closed 2 years ago

zardoy commented 2 years ago

I see you just started printing the body of the snippet. This PR improves this preview.

Now snippets preview look like this: image

With this we align our behaviour with how builtin vscode snippet completions works.

I've been using it for a while and can say that it works pretty well (even though the package is outdated). The only thing that it can't handle properly is transform part in snippets e.g. ${1/(.*)/${1:/capitalize}/}].

Also note as we started printing the body this preview can be really big. As an example await with 10 lines of call expression.

ipatalas commented 2 years ago

Hmm, what's the benefit?

It works pretty well now: image

Yes, it does show cursor snippets ($1 and so on) but I actually find it quite useful.

As for big previews, well, a quick example from one of other issues looks pretty good too: image

zardoy commented 2 years ago

It works pretty well now:

Yes, but with this tabstops (on this example) its hard to tell what inserted text would look like. I just got used to builtin vscode snippets, they always give more "clean" result in preview.

However, I just used it for a while and can say that you're probably right. Currently preview is more informative. Also on some snippets like $1 ? {{expr}} : $2 it would look stupid, so I don't mind, lets close it.

zardoy commented 2 years ago

Also, I saw that you are using custom description for builtin postfixes to just not include tabstops. I also opened this PR to align that behavior, because now:

So in my opinion we should either:

What do you think?

ipatalas commented 2 years ago

Yeah, makes sense to go either way but be consistent. I have just published new version of the extension because there's already quite few things worth releasing. I'll take a look on that and the other issues in the upcoming days.

zardoy commented 2 years ago

Reopening so you don't forget about it :)

ipatalas commented 2 years ago

Ok, did a quick PoC and here's the result: image The one at the top is the old one and the new one on the second line to see the difference. I kind of like it but still I feel it might be little bit too cluttered for some of the users so I'm considering making a feature flag for that so that it can be toggled in the settings.

Especially there's this: https://code.visualstudio.com/docs/editor/userdefinedsnippets#_variables When it's used it gets little bit tricky: image In theory it's correct because that's the body of the snippet but it's not easy to understand. However this can only (for now) be used in custom snippets so user should be aware and I think it would make sense to add meaningful description for the snippet itself and then I'd include that as the first line more or less like on the above screen. Ideally I would like it to be a preview of what's gonna happen in this specific context but I'm not sure VS Code API allows that (evaluation of the snippet before it happens)

zardoy commented 2 years ago

So am I free to add setting that will control rendering format of builtin and custom snippets? In this case, I think I'll also refactor builtin "description" of builtin snippets.

Especially there's this: code.visualstudio.com/docs/editor/userdefinedsnippets#_variables

You check how builtin vscode snippets work, they also skip rendering of variables.

ipatalas commented 2 years ago

So am I free to add setting that will control rendering format of builtin and custom snippets? In this case, I think I'll also refactor builtin "description" of builtin snippets.

I've already refactored built-in description yesterday: https://github.com/ipatalas/vscode-postfix-ts/commit/dc3d7a036ea6d2b7ffa08bb997b4299f1ae7965d

image

I didn't have good idea for the description so I just left the replacement alone for now. If you have idea for that let me know.

As for the custom snippets if the description field is provided then it will be displayed before the replacement code: image

Especially there's this: code.visualstudio.com/docs/editor/userdefinedsnippets#_variables

You check how builtin vscode snippets work, they also skip rendering of variables.

True, I had a look on that yesterday and you're right. I started to like that idea so probably this clean version should be the default not to confuse people. The other one we can consider "power user mode" because one has to know how those tabstops work to be able to actually use that knowledge somehow and I would have no idea if I wasn't the author of VSCode extensions so I guess most people are not fully aware.

However, that SnippetParser seems to be no longer maintained and it doesn't support all the syntax so for instance it doesn't make this any cleaner: image

zardoy commented 2 years ago

Hey! I just implemented the setting check this out!

However, that SnippetParser seems to be no longer maintained and ...

Yeah, I stated this in the body, this seems to be only a case where it fails . Ideally we should have this method builtin..