Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add local parser building functionality #220

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ethan-leba
Copy link

This PR will make it possible for users to build TS grammars locally with no external CLI dependencies, which is a big win for making that an accessible option for users.

A couple points of discussion:

  1. Pulling in tree-sitter-cli brings in a non-trivial amount of unrelated cli-based dependencies in order to use it's tree-sitter generate functionality. Do we think it's worth it?
  2. Building a parser takes about 3s on my machine, more so if we're generating as well. Do we want to consider some sort of asynchronous build option?

I'm open to any and all changes in order to make this PR easier to merge/ fit better with the codebase!

If/when this gets merged, we'd likely also want to rework tree-sitter-langs to use this function.

@ethan-leba ethan-leba marked this pull request as ready for review April 27, 2022 15:50
Comment on lines +466 to +469
DST-PATH specifies whether the build result should be placed. If
DST-PATH is unset, it will default to the value of
`tree-sitter-langs-grammar-dir' if bound. Otherwise, an error will be raised."
(if-let ((dst-path (or dst-path (bound-and-true-p tree-sitter-langs-grammar-dir))))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels a bit hacky to me, but I think users will want the build directory to default to the TSL grammar dir 99% of the time

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about improving the situation with something like this:

(defvar tsc-dst-path-decider-func #'tsc-default-dst-path-decider
  "Function to decide the destination path for building the parser. 
Takes `src-path` and `dst-path` as arguments, should return the final `dst-path` to use.")

(defun tsc-default-dst-path-decider (src-path dst-path)
  "Default function for deciding destination path."
  (or dst-path (bound-and-true-p tree-sitter-langs-grammar-dir)))

(cl-defun tsc-build-parser-from-source (src-path &key generate dst-path)
;;;;
   (if-let ((dst-path (funcall tsc-dst-path-decider-func src-path dst-path)))

@ethan-leba
Copy link
Author

@ubolonton there appears to be a regression with windows CI unrelated to this PR, tested with an empty commit here: #221

@ubolonton
Copy link
Collaborator

Windows CI used to pin Emacs version, but no longer does, after this supposedly temporary change.

This likely means the test hl::face-mapping fails on Emacs 28.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants