Skip to content

Tests for more Clojure tags #4126

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

Merged
merged 2 commits into from
May 20, 2025
Merged

Conversation

aartaka
Copy link
Contributor

@aartaka aartaka commented Nov 21, 2024

Hi! Here's some Clojure tests for #4123. That's not all, but it's the most frequently used things—vars and multimethods.

@masatake
Copy link
Member

masatake commented Dec 3, 2024

Thank you for making this pull request. I will merge this with some revising.

[& _]
nil)

(defmethod test :test2 named-method
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain this line?
I wonder why named-method should be extracted because my knowledge of Clojure is limited.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a method-specific name that's added to multimethod function name. See https://clojuredocs.org/clojure.core/defmethod#example-542692c7c026201cdc3269cd

Copy link
Member

Choose a reason for hiding this comment

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

I need help understanding.
Can named-method be used with a function?
I guess, with the name, a user can call the method by passing multi-method-dispatching. If my guessing is correct,I think 'named-method' should be tagged as a function, not a method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it doesn't work as a self-sufficient function. It's merely a piece of metadata identifying the method. It's only useful in debugging and (given ctags support) in location search.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Thank you. We should introduce a new kind for such a language object. What do Clojure hackers call it? I think of method-identifier/i or method-id/i.

The ideal tags for named-method looks like:

named-method	input.clj	/^(defmethod test :test2 named-method$/;"	i	multimethod.test	method:test
test	input.clj	/^(defmethod test :test2 named-method$/;"	m	multimethod.test	identifier:named-method

Anyway, the current Clojure parser cannot extract caddr.
So, to satisfy these new test cases, we must rewrite the parser.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for disappearing 😔 Why not named method? But yes, method-id(endifier) is fine by me too!

Copy link
Member

Choose a reason for hiding this comment

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

If named-method is tagged as a method, a client tool like Vim may show it as a candidate in input completion.

No, it doesn't work as a self-sufficient function.

However, it is not callable, as you wrote. So, named-method should not be shown on the candidate list. If named-method is tagged as a method-id, Vim may not show as far as Vim shows only method kind tags.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, all fair!

@masatake masatake added this to the 6.2 milestone Dec 11, 2024
@masatake masatake force-pushed the more-clojure-tags branch from 01fc488 to e20c573 Compare May 19, 2025 20:26
@masatake masatake force-pushed the more-clojure-tags branch from e20c573 to c413590 Compare May 19, 2025 20:26
@masatake masatake force-pushed the more-clojure-tags branch from c413590 to b766cb3 Compare May 19, 2025 21:00
Copy link

codecov bot commented May 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.85%. Comparing base (123588e) to head (b766cb3).
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4126   +/-   ##
=======================================
  Coverage   85.85%   85.85%           
=======================================
  Files         244      244           
  Lines       62986    62986           
=======================================
  Hits        54079    54079           
  Misses       8907     8907           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@masatake masatake merged commit b28ac7c into universal-ctags:master May 20, 2025
82 checks passed
@masatake
Copy link
Member

@aartaka Thank you for contributing.

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

Successfully merging this pull request may close these issues.

2 participants