Actually, I'll turn this into a draft for now, it isn't really ready for review yet.

I need some guidance/opinions on the api for AST-nodes.

I currently favor having the nodes just be flat tables (+__tostring) with documented fields (this could be the specification for a choice:

A Choice contains the fields
- {tabstop} (number) Position of the choice ("${<tabstop>|choice1,choice2,choice3|}")
- {items} (string[]) The different choices in order of appearance ("${1|<items[1]>,<items[2]>,...,<items[n]>}")

) but this could make it harder to be backward-compatible if (and that's a very big, unlikely if, in my judgment) the lsp-specification for snippets changes in a not-backward-compatible way.

@hrsh7th I'd love to hear your thoughts on the API

How are spec-changes handled in other modules?

Just found microsoft/language-server-protocol#303 (comment), I guess we won't need to worry about breaking changes :)

Okay, I think this can be looked at :D

The parser lives at vim.lsp.parser for now (still unsure if that is really the place for it), the various constructors for snippet, placeholder etc. in vim.lsp.parser.ast.

The parser lives at vim.lsp.parser for now (still unsure if that is really the place for it), the various constructors for snippet, placeholder etc. in vim.lsp.parser.ast.

To be honest, I still think vim.lsp.snippet is the right place for it -- especially in light of #16499. We don't need a new module just for a single function (parse()), and it makes sense to bundle everything snippet-related in vim.lsp.snippet. (If anything, we might want to move non-LSP related snippet functionality to vim.snippet if there's a broader use for it, but let's keep everything minimal and contained for now.)

I'm also not 100% happy with snippet.ast, although I don't have a concrete better suggestion (snippet.tree? snippet.parsed?). But we already have a similar API: the tree-sitter languagetree, and it would be helpful if the API for the snippet tree was modeled after that one as much as possible and reasonable.

To be honest, I still think vim.lsp.snippet is the right place for it -- especially in light of #16499.

Ah, so vim.lsp.snippet would have parse and the ast? That sounds alright

I'm also not 100% happy with snippet.ast, although I don't have a concrete better suggestion (snippet.tree? snippet.parsed?).

I share the sentiment, but can't think of anything better as well :/

But we already have a similar API: the tree-sitter languagetree, and it would be helpful if the API for the snippet tree was modeled after that one as much as possible and reasonable.

eg. new+getters/setters instead of just raw access to most fields?

I'm wondering if neovim should document the internal AST that's from parses and creates the snippet syntax defined in the LSP specification.

So in the PR, I chose a mechanism that customize the features by overriding the function as like vim.ui.input .

(Of course, in case we try to define AST spec, I can help for it)

I'm wondering if neovim should document the internal AST that's from parses and creates the snippet syntax defined in the LSP specification.

It should be enough to show which field of an ast-node corresponds to which part of the snippet, explaining how they should interact would just be repeating the official spec I think.

So in the PR, I chose a mechanism that customize the features by overriding the function as like vim.ui.input .

Ahh, okay that's pretty nice

Tangentially, there's a few small bugs in the current implementation (:+ and :- are parsed as the same, ${1:CaptureDoesNotExistsText} is missing, empty if/else_text breaks the parser (discovered by @kmarius)).
I think I can fix those, should I tack them on here or address them in a separate PR?

I think the bug should be fixed with some of the tests.
IMO, it should be a separated PR but I don't have strong opinion.
(Thank you for pointed me out.)

Tangentially, there's a few small bugs in the current implementation (:+ and :- are parsed as the same, ${1:CaptureDoesNotExistsText} is missing, empty if/else_text breaks the parser (discovered by @kmarius)).
I think I can fix those, should I tack them on here or address them in a separate PR?

Depends on how they interact with your changes -- if they're independent, a separate PR is better as it can be merged quickly (and backported) -- as a bonus, this will then lose you your "first contributor" status so you don't need constant approval for running CI on your other more complex PRs ;)

The bugs cab be fixed independently of this PR, I'll open another one

this will then lose you your "first contributor" status so you don't need constant approval for running CI on your other more complex PRs ;)

Truue, nice :)

09d7802 makes the ast-api more like languagetree

1. each node has its own metatable (which can be useful for adding some functions, eg. format:apply(captures) or transform:apply(text, captures) (but I can't think of any beside those, most likely apply_transform would be enough), and it makes __tostring a bit nicer IMO)
2. use new: this makes creating nodes more verbose (see snippet.lua), I don't think it's really necessary. I'd prefer just Nodename(args).

Should it stay like this?

Closing this as we've vim.snippet now. It doesn't expose a parse yet, that's something to consider after the points in #25696 are solved, to avoid having to account for AST stability/bwc already.