emacs-tree-sitter / elisp-tree-sitter

Emacs Lisp bindings for tree-sitter
https://emacs-tree-sitter.github.io
MIT License
816 stars 73 forks source link

Make tree-sitter-node-at-pos accept special node-type arguments :named and :anonymous #171

Closed rolandwalker closed 2 years ago

rolandwalker commented 2 years ago

xref #169

This PR teaches tree-sitter-node-at-pos to accept special NODE-TYPE arguments of :named and :anonymous, which return the smallest named or smallest anonymous syntax nodes at POS, respectively.

The quoted nil 'nil in pcase looks odd, but is correct.

This defensive coding might not be needed: (when (and node (stringp (tsc-node-type node))) node).

I'm not sure if this is helpful and correct documentation, as written:

Whenever NODE-TYPE is non-nil, it is possible for the function to return nil.

Can the function return nil when NODE-TYPE is nil? Edit: I mean, in a tree-sitter buffer?

I noticed FIX: Signal an error for non-existing node types in the test file. What needs to be done there?

ubolonton commented 2 years ago

This defensive coding might not be needed: (when (and node (stringp (tsc-node-type node))) node).

Yeah, node should already be non-nil, unless there's a bug in tsc-get-descendant-for-position-range.

I'm not sure if this is helpful and correct documentation, as written:

Whenever NODE-TYPE is non-nil, it is possible for the function to return nil.

This is correct, except for :named (because the root node is named). I think it's helpful to remind users that the function can return nil.

Can the function return nil when NODE-TYPE is nil? Edit: I mean, in a tree-sitter buffer?

No, AFAICT, because the root node covers the whole buffer.

I noticed FIX: Signal an error for non-existing node types in the test file. What needs to be done there?

tree-sitter-node-at-pos should be made to signal an error when NODE-TYPE is not in the grammar. I'm still not sure whether there should be a corresponding parameter to toggle that. It could be a separate PR, though.

rolandwalker commented 2 years ago

Yeah, node should already be non-nil

Fixed.

This is correct, except for :named

Fixed.