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

Discover/link positional [de]construction #104

Open
blackgnezdo opened this issue Sep 27, 2019 · 2 comments
Open

Discover/link positional [de]construction #104

blackgnezdo opened this issue Sep 27, 2019 · 2 comments

Comments

@blackgnezdo
Copy link
Contributor

blackgnezdo commented Sep 27, 2019

Given a code like this it would be great to get cross-references:

data Foo = Foo { x :: Int, y :: Double }

foo = Foo 1 2

bar (Foo x _) = x

E.g., when one clicks on x in data definition both foo and bar are useful places to link to because they "use" x. For bonus points it would be great to not link _ in bar when y is clicked.

As a workaround, clicking on the data constructor name currently works, so this issue is a pure usability enhancement, not a particularly critical enabler.

@robinp
Copy link
Collaborator

robinp commented Sep 28, 2019

I added some comments on the slightly related #19 .

A quick summary of the current state after quickly checking https://github.com/google/haskell-indexer/blob/master/kythe-verification/testdata/basic/RecordReadRef.hs - correct me if it is wrong:

  1. now a non-literal deconstruction like bar (Foo a _) = ... will only emit a binding for a, and connect the usages of a to it. There isn't any edge going from either the binding span or the ref spans to the field. (this is why @blackgnezdo opened the issue).

  2. explicitly named deconstruction like bar Foo { x = a } = ... will connect x to the field binding. Usages of a won't be listed in the backrefs for field x.

  3. punned deconstruction like bar Foo {x} won't introduce a local binding (?), both the pattern and the usages will directly reference the field binding. Which is a bit odd highlight-wise and semantically (say, all x refs in the file are hilit even when hovering a local usage in a single function?), but at least brings the usages in the backref.

  4. For non-record constructor data Bar = Bar Int, not sure - maybe we don't even emit a field decl at the span of Int (?), so implicit matches don't even have the target to reference.

IMO, a user generally wants to see how a field is used in a specific context when looking through the backreferences. The various cases are:

  • current punning refs in (3) are not semantically nice, but bring the backref

  • for (1), (2) and (4), adding an edge from the pattern-matched name to the field would bring them in the backrefs, but if the pattern name would differ from the field name, the listing could look odd (the UI can solve this by bolding the reference in the listed line to resolve ambiguity)

As a unified solution, my gut feeling is that a local usage of a deconstructed var should directly reference both the local binding and the generic field binding in the data definition.

@robinp
Copy link
Collaborator

robinp commented Oct 3, 2019

See discussion in kythe/kythe#3934 - TLDR the local binding should bind a variable/import node (maybe the subkind is not important, could be left or be field as well?), which should have an aliases edge towards the generic field binding. The index preprocessor (with appropriate config) will merge the backrefs for the local usage into the field's usage.

/cc @zrlk to verify I understand correctly

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

No branches or pull requests

2 participants