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 checks for tree-sitter definitions #14

Merged
merged 1 commit into from
Jun 15, 2023

Conversation

basil-conto
Copy link
Contributor

@basil-conto basil-conto commented Jun 11, 2023

This is an initial attempt to check the following Emacs 29 additions:

  • treesit-defun-type-regexp
  • treesit-font-lock-rules
  • treesit-sentence-type-regexp
  • treesit-sexp-type-regexp
  • treesit-simple-imenu-settings
  • treesit-simple-indent-rules

Marking the PR as draft because testing is incomplete.
I should be able to finish up with testing this week.

In the meantime I was hoping to get feedback on whether:

  • these additions are welcome
  • the coding style is acceptable
  • I'm on the right track in general

and suggestions for improvements.

BTW, these checks have already been run against emacs-29:
the log is attached in PR mattiase/xr#6.

Thanks!

Copy link
Owner

@mattiase mattiase left a comment

Choose a reason for hiding this comment

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

This commit is fine: scanning another known variable that is used in several files increases the relint catchment to a useful degree, and there seems to be no risk for false positives. (No true positives either so far, but it certainly won't hurt.)

I would probably have done without the new defconst since it's just two names instead of one, but you don't need to change that.

@mattiase
Copy link
Owner

That previous comment was just about the first commit. The other one is more substantive and I haven't looked through it yet.

relint.el Outdated Show resolved Hide resolved
@mattiase
Copy link
Owner

There seem to be at least one error that relint should have caught but didn't (ruby-ts-mode.el:1024). That indicates a need to traverse arguments of at least treesit-query-compile and treesit-query-capture.

@basil-conto basil-conto force-pushed the blc/treesit branch 2 times, most recently from ab78f44 to 16c5906 Compare June 14, 2023 22:06
@basil-conto
Copy link
Contributor Author

There seem to be at least one error that relint should have caught but didn't (ruby-ts-mode.el:1024). That indicates a need to traverse arguments of at least treesit-query-compile and treesit-query-capture.

Well noticed. I've now added what I think is pretty complete coverage of the relevant treesit API in Emacs 29.

Will you report/fix the ruby-ts-mode issue upstream, or would you like me to?

@mattiase
Copy link
Owner

I've now added what I think is pretty complete coverage of the relevant treesit API in Emacs 29.

Nice, did it catch anything else?

Will you report/fix the ruby-ts-mode issue upstream, or would you like me to?

Please do.

@basil-conto
Copy link
Contributor Author

I've now added what I think is pretty complete coverage of the relevant treesit API in Emacs 29.

Nice, did it catch anything else?

No, only the ruby-ts-mode issue.

Will you report/fix the ruby-ts-mode issue upstream, or would you like me to?

Please do.

Done: https://bugs.gnu.org/64019#50

@basil-conto basil-conto marked this pull request as ready for review June 15, 2023 17:20
@mattiase mattiase merged commit a37c060 into mattiase:master Jun 15, 2023
@mattiase
Copy link
Owner

Excellent, thank you very much!

@mattiase
Copy link
Owner

Building on your work, I improved the accuracy for treesit query regexps a little.

@basil-conto basil-conto deleted the blc/treesit branch June 16, 2023 10:48
@basil-conto
Copy link
Contributor Author

Building on your work, I improved the accuracy for treesit query regexps a little.

Excellent, thank you very much!

dickmao pushed a commit to commercial-emacs/commercial-emacs that referenced this pull request Jun 17, 2023
These issues were caught by modified versions of the GNU ELPA
packages xr and relint:
- mattiase/xr#6
- mattiase/relint#14

* lisp/gnus/gnus-art.el (gnus-parse-news-url): Remove redundant
numbered group and calls to match-string.
* lisp/progmodes/c-ts-mode.el (c-ts-mode--c-or-c++-regexp): Fix shy
group mistyped as optional colon (bug#64019#29).
* lisp/vc/vc-git.el (vc-git-annotate-time): Ditto.  Also fix
timezone parsing by using iso8601-parse (bug#64069).
* test/lisp/vc/vc-git-tests.el (vc-git-test-annotate-time): New
test.
jollaitbot pushed a commit to sailfishos-mirror/emacs that referenced this pull request Jun 17, 2023
The shy groups were caught by modified versions of the GNU ELPA
packages xr and relint:
- mattiase/xr#6
- mattiase/relint#14

* lisp/progmodes/ruby-ts-mode.el (ruby-ts--s-p-query): Quote special
character in regexp.
* lisp/progmodes/java-ts-mode.el (java-ts-mode--font-lock-settings):
* lisp/progmodes/js.el (js--plain-method-re):
(js--treesit-font-lock-settings):
* lisp/progmodes/rust-ts-mode.el (rust-ts-mode--font-lock-settings):
* lisp/progmodes/typescript-ts-mode.el
(typescript-ts-mode--font-lock-settings): Replace character
alternative [\\d], which matches '\' or 'd', with the most likely
intention [0-9].  Fix shy groups mistyped as optional colons.
Remove unneeded numbered :match group in rust-ts-mode (bug#64019).
dickmao pushed a commit to commercial-emacs/commercial-emacs that referenced this pull request Jun 17, 2023
This was originally installed on 2023-06-17 in the emacs-29 release
branch and later reverted.  The intention is to backport it after
Emacs 29.1 is released.

The shy groups were caught by modified versions of the GNU ELPA
packages xr and relint:
- mattiase/xr#6
- mattiase/relint#14

* lisp/progmodes/ruby-ts-mode.el (ruby-ts--s-p-query): Quote special
character in regexp.
* lisp/progmodes/java-ts-mode.el (java-ts-mode--font-lock-settings):
* lisp/progmodes/js.el (js--plain-method-re):
(js--treesit-font-lock-settings):
* lisp/progmodes/rust-ts-mode.el (rust-ts-mode--font-lock-settings):
* lisp/progmodes/typescript-ts-mode.el
(typescript-ts-mode--font-lock-settings): Replace character
alternative [\\d], which matches '\' or 'd', with the most likely
intention [0-9].  Fix shy groups mistyped as optional colons.
Remove unneeded numbered :match group in rust-ts-mode (bug#64019).
veden pushed a commit to veden/emacs that referenced this pull request Jul 30, 2023
This was originally installed on 2023-06-17 in the emacs-29 release
branch and later reverted.  This backport follows the Emacs 29.1
release (bug#64019).

The shy groups were caught by modified versions of the GNU ELPA
packages xr and relint:
- mattiase/xr#6
- mattiase/relint#14

* lisp/progmodes/ruby-ts-mode.el (ruby-ts--s-p-query): Quote special
character in regexp.
* lisp/progmodes/java-ts-mode.el (java-ts-mode--font-lock-settings):
* lisp/progmodes/js.el (js--plain-method-re):
(js--treesit-font-lock-settings):
* lisp/progmodes/rust-ts-mode.el (rust-ts-mode--font-lock-settings):
* lisp/progmodes/typescript-ts-mode.el
(typescript-ts-mode--font-lock-settings): Replace character
alternative [\\d], which matches '\' or 'd', with the most likely
intention [0-9].  Fix shy groups mistyped as optional colons.
Remove unneeded numbered :match group in rust-ts-mode.

(cherry picked from commit cd8d3f3)
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.

2 participants