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

Recast tree-sitter-node-at-point as tree-sitter-node-at-pos, adding optional POS parameter #169

Closed rolandwalker closed 2 years ago

rolandwalker commented 2 years ago

You might reasonably object to adding a POS parameter to a defun named *-at-point.

In which case, is a tree-sitter-node-at-pos welcome? My purpose is to avoid needless save-excursions in the calling code.

rolandwalker commented 2 years ago

Also, why does the function use equal instead of eq?

ubolonton commented 2 years ago

In which case, is a tree-sitter-node-at-pos welcome? My purpose is to avoid needless save-excursions in the calling code.

Yes, tree-sitter-node-at-pos is better. Please keep tree-sitter-node-at-point as an obsolete alias though.

Also, why does the function use equal instead of eq?

Node types can be strings (anonymous nodes), so equal would be wrong.

rolandwalker commented 2 years ago

Yes, tree-sitter-node-at-pos is better. Please keep tree-sitter-node-at-point as an obsolete alias though.

Done, with autoload cookie on the alias. I don't know org-mode, so please check how I changed the doc.

Node types can be strings (anonymous nodes)

Ah. Then I wonder if there should be a separate defun which only finds non-anonymous nodes, or whether it makes sense to make the interface more complicated with a special value for NODE-TYPE like

(defun tree-sitter-node-at-pos (&optional node-type pos)
  "Return the smallest syntax node of type NODE-TYPE at POS.
If NODE-TYPE is nil, return the smallest syntax node at POS.
If NODE-TYPE has the special value `:named', return the smallest
named (non-anonymous) syntax node at POS.  IF POS is nil,
defaults to the point."
  (let* ((root (tsc-root-node tree-sitter-tree))
         (p (or pos (point)))
         (node (tsc-get-descendant-for-position-range root p p)))
    (if node-type
        (let ((this node) result)
          (while this
            (if (or (equal node-type (tsc-node-type this))
                    (and (eq :named node-type)
                         (symbolp (tsc-node-type this))))
                (setq result this
                      this nil)
              (setq this (tsc-get-parent this))))
          result)
      node)))

That form could support a special type :anonymous as well.

ubolonton commented 2 years ago

Done, with autoload cookie on the alias. I don't know org-mode, so please check how I changed the doc.

Don't worry. That line in the doc is commented out, i.e. this function is not yet documented there. If you know markdown, you can add a note in CHANGELOG.md though. (If not, I'll update it later.)

Ah. Then I wonder if there should be a separate defun which only finds non-anonymous nodes, or whether it makes sense to make the interface more complicated with a special value for NODE-TYPE like ... That form could support a special type :anonymous as well.

Yeah, special values for NODE-TYPE are fine. Keywords are already used for special node types (see the docstring of tsc-lang-node-type).

Implementation-wise, they should be outside of the upward traversal, though. For :named, returning the result of calling tsc-get-named-descendant-for-position-range should be enough. For :anonymous, if the smallest node is named, none of its ancestors can be anonymous.

rolandwalker commented 2 years ago

you can add a note in CHANGELOG.md

Done

Yeah, special values for NODE-TYPE are fine.

Great. I may try that in a separate PR.