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

Multiple hits for function definition using schema #85

Closed
expez opened this issue Jun 12, 2015 · 5 comments
Closed

Multiple hits for function definition using schema #85

expez opened this issue Jun 12, 2015 · 5 comments
Labels

Comments

@expez
Copy link
Member

expez commented Jun 12, 2015

(ns demo.greeter
  (:require [schema.core :as s]))

(s/defn greet [person :- s/Str]
  (println "Hello, " person))

This resulted in the following hits:

C:\git\demo\src\demo\dependent.clj:2: (:require [demo.greeter :refer [greet]]
C:\git\demo\src\demo\dependent.clj:6: (greet "Hello" "Tom"))
C:\git\demo\src\demo\greeter.clj:4: (s/defn greet [person :- s/Str]
C:\git\demo\src\demo\greeter.clj:4: (s/defn greet [person :- s/Str]

The two final hits should be a single hit. Most likely this is a problem with the filter of symbol occurrences in macro definitions.

@expez expez added the bug label Jun 12, 2015
@benedekfazekas
Copy link
Member

this is basically a macro related problem apparently. s/defn is a macro and our trick in refactor-nrepl.find.find-symbol/dissoc-macro-nodes to filter macros out does not work on it as it does contain the var name before macro expansion.

@expez
Copy link
Member Author

expez commented Sep 26, 2015

Can we handle this case specially, given the popularity of schema? Perhaps there's a reliable way to separate the spurious matches from the real ones?

@expez
Copy link
Member Author

expez commented Oct 11, 2015

I'm not really satisfied with the current solution. We're now unable to report (or count) occurrences on the same line.

I think a better approach is to verify suspicious occurrences in the middleware, before passing them on to the client.

We can do this by checking if the symbol actually occurs at the coordinates in question.

@benedekfazekas
Copy link
Member

it does not make sense to report occurrences on the same line in grep buffer. that does not mean that verify occurrences is a bad idea of course. I would deem this low priority tho as the current approach works for all cases i am aware of

@expez
Copy link
Member Author

expez commented Oct 12, 2015

it does not make sense to report occurrences on the same line in grep buffer.

I'm not sure I agree with this, but what I value more is consistency. So far I've tried using ag and it displays only the first hit on a line, but the total count includes all matches. I think a single line would work better if we could highlight the matches, somehow. That way you'd see that there are multiple hits on one line more easily.

Anyway, we're in a somewhat tricky situation in that the client does know if it's dealing with a real occurrence or a spurious one. So in some cases we're hiding duplicates (and can't report a correct count) but in other cases we have the correct count and it would be nice to report it.

@expez expez closed this as completed in 2cba735 Oct 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants