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

(crafted-ide-configure-tree-sitter) currently broken due to issues in treesit-auto #427

Open
3 tasks
dev-zero opened this issue Jan 11, 2025 · 9 comments
Open
3 tasks
Labels
bug Something isn't working

Comments

@dev-zero
Copy link

Operating System

Linux/BSD

Additional Operating System information

No response

Emacs Version

29

Emacs Configuration Details

  • Native Compilation
  • pGTK
  • alternative package manager (use-package, straight.el, ...)

Anything else that may be related to the issue you're having?

No response

What happened?

Enabling (crafted-ide-configure-tree-sitter) should install all "default" tree-sitter grammars via treesit-auto.
Due to changes in packaging (latex), mistake in naming (janet) or changes in directory layout (markdown) this leads to a repeated warning on Emacs startup. See renzmann/treesit-auto#116 for the upstream issue and the referenced issues accordingly.

What should have happened?

The grammars should install properly.

@dev-zero dev-zero added the bug Something isn't working label Jan 11, 2025
@jvdydev
Copy link
Contributor

jvdydev commented Jan 12, 2025

Thank you for reporting the issue.

From the user side, this can (temporarily) be fixed using the opt-in argument together with treesit-auto-langs, by removing janet, latex and markdown from the list and passing it to crafted-ide-configure-tree-sitter.

;; using seq-remove
(crafted-ide-configure-tree-sitter
 (seq-remove (lambda (a)
               (member a '(janet markdown latex)))
             treesit-auto-langs))

;; using cl
(require 'cl)
(crafted-ide-configure-tree-sitter
 (cl-remove-if (lambda (a)
               (member a '(janet markdown latex)))
             treesit-auto-langs))

Since crafted-ide-configure-tree-sitter only customizes the treesit-auto-langs variable, it is also possible to do:

(customize-set-variable 'treesit-auto-langs
                        (seq-remove (lambda (a)
                                      (member a '(janet markdown latex)))
                                    treesit-auto-langs))
;; (cl-remove-if version omitted)

I'm not entirely sure how we want to go about fixing this inside crafted-emacs.

I've looked through the treesit-auto repository, seems like there hasn't been much movement since ~April last year, even though there are multiple PRs open fixing this problem.

We could obviously remove them inside crafted-ide-configure-tree-sitter (with the same mechanism as above), however:
For janet, this is fine as there is (to my knowledge) no working version of janet grammar.
As for the other two, they would be removed even after upstream gets fixed, more precisely until we remove the "fix".
It would also would (probably?) break at least markdown/latex for people who already have an older built version of these libraries (in the sense it would no longer load a working version).

Any other ideas @jeffbowman @sthesing ?

@jeffbowman
Copy link
Contributor

Been busy at work, sorry for the delay. The last commit I see was in May last year when the author of treesit-auto bumped the revision. There are other forks which have more recent updates (notably the fork by gs-101).

Short of waiting for the author of treesit-auto to fix things, we could potentially provide our own implementation internally. That is a lot of extra burden on this project to maintain though.

Another possible answer is to just remove treesit-auto from crafted-emacs and use built-in facilities. That would mean a bit of thought and a rewrite of all of the treesit stuff, but long term it may be the better answer.

@dev-zero
Copy link
Author

Thank you for reporting the issue.

Thanks for the quick reply.

From the user side, this can (temporarily) be fixed using the opt-in argument together with treesit-auto-langs, by removing janet, latex and markdown from the list and passing it to crafted-ide-configure-tree-sitter.

;; using seq-remove
(crafted-ide-configure-tree-sitter
 (seq-remove (lambda (a)
               (member a '(janet markdown latex)))
             treesit-auto-langs))

This gives me:

⛔ Warning (initialization): An error occurred while loading ‘/Users/tiziano/.emacs.d/init.el’:

Symbol's value as variable is void: treesit-auto-langs

To ensure normal operation, you should investigate and remove the
cause of the error in your initialization file.  Start Emacs with
the ‘--debug-init’ option to view a complete error backtrace.

@jvdydev
Copy link
Contributor

jvdydev commented Jan 14, 2025

My bad, probably needs to require treesit-auto first, as it's not loaded yet (so the symbols from the library are not known to Emacs yet):

(require 'treesit-auto)
(crafted-ide-configure-tree-sitter
 (seq-remove (lambda (a)
               (member a '(janet markdown latex)))
             treesit-auto-langs))

A more "complete" version may also give appropriate error:

(if (require 'treesit-auto nil :noerror)
    (crafted-ide-configure-tree-sitter
     (seq-remove (lambda (a)
                   (member a '(janet markdown latex)))
                 treesit-auto-langs))
  (warn "treesit-auto not available"))

We also normally check if Emacs is compiled with treesitter support (see the full implementation, although this is unrelated to your issue, just for completeness 😄).

@jeffbowman
Copy link
Contributor

I've been thinking about this for a couple of days... one of the reasons we provide the opt-in parameter is to allow the user to install only the grammars they actually use, rather than all of them. One other approach would be to remove the three languages which are failing from your opt-in list. Here is what my configuration looks like:

(crafted-ide-configure-tree-sitter '(go java javascript typescript))

Using built-in facilities, it may be possible to do something like this (see below) for latex, markdown, and janet:

(dolist (grammar '((tsx . ("https://github.com/tree-sitter/tree-sitter-typescript" "v0.23.2" "tsx/src"))
                   (typescript . ("https://github.com/tree-sitter/tree-sitter-typescript" "v0.23.2" "typescript/src"))))
  (unless (alist-get (car grammar) treesit-language-source-alist)
    (add-to-list 'treesit-language-source-alist grammar))
  (unless (treesit-language-available-p (car grammar))
    (treesit-install-language-grammar (car grammar)))
  (dolist (mapping '((typescript-mode . typescript-ts-base-mode)
                     (js-mode . typescript-ts-base-mode)
                     (js2-mode . typescript-ts-base-mode)))
    (unless (alist-get (car mapping) major-mode-remap-alist)
      (add-to-list 'major-mode-remap-alist mapping))))

obviously, change the grammar urls appropriately as well as the mode mapping.

@sthesing
Copy link
Contributor

So is this something we need to address in code? Or in documentation?

@jeffbowman
Copy link
Contributor

So is this something we need to address in code? Or in documentation?

The problem is outside Crafted Emacs. The code presented here is a work-around to try to help those who might run into the problem. Unless we come up with our own hand-rolled auto installer for treesitter grammars, this isn't really a Crafted Emacs specific problem.

@sthesing
Copy link
Contributor

Sorry Jeff, I phrased the question poorly. What I mean is:

  1. Do we do something about it or is this issue enough?
  2. If we do something about it, do we do it by adding the logic of your example into our code or do shall I put some directions into the documentation?

Now, you already answered question No 2. Thanks for that. But what do you folks think? Is this something that should go into the docs? Or is this issue enough? Or do we need to add more general explanation to the documentation?
I'm on the fence here.

@jeffbowman
Copy link
Contributor

I'm happy enough to just leave it here in the comments. We may think about adding something related to this in a general sense. Along the lines of: we appreciate when people report issues with packages we include in our configuration, and we will try to help as much as possible, but for issues outside of Crafted Emacs, we will defer to the users best judgement on managing their own configuration. Anything that can be directly attributed to code we wrote we will do our best to resolve.

Does that make sense? Maybe it belongs in the contributing section.

Happy to have more discussion on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants