Skip to content

Go to Import could become Go to Directive and inform part files #59703

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

Open
FMorschel opened this issue Dec 12, 2024 · 12 comments
Open

Go to Import could become Go to Directive and inform part files #59703

FMorschel opened this issue Dec 12, 2024 · 12 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. P4 type-design

Comments

@FMorschel
Copy link
Contributor

FMorschel commented Dec 12, 2024

After #56584 is complete and working. I was wondering whether it would make sense for it to also show the part directive where a declaration is coming from. For the enhanced parts feature parts are now more complex and can have inner parts with more things so I think this tool would be great for this.

@DanTup, not sure what sould be done in this case (if we'd need to replace the text on the context menu) if this is ever implemented just so you are aware of this.

We could also make a new call that would use the same logic but for part files and that would be a new thing but I don't think that is the best experience (the user would need to know if that was either imported or implemented in a part).

@FMorschel FMorschel added the legacy-area-analyzer Use area-devexp instead. label Dec 12, 2024
@DanTup
Copy link
Collaborator

DanTup commented Dec 12, 2024

I think it would be reasonable to include parts.

Whether we should change the text though, I'm less sure. I feel like "Imports" might make it more obvious what this does (even if it's not strictly imports) and we already use "Imports" in some other places like "Organize Imports" where we really mean all directives.

We could also write something like "Go to Import/Part Directives" to keep the word "import" while being more correct.

@bwilkerson
Copy link
Member

I'm not sure I understand what the goal is for this.

I believe the goal for 'Go to Import' is to answer the question "where is this name imported from". A developer presumably wants to know that in order to control the names that are in scope (via hide and show), especially when importing libraries they don't control, and knowing which import directives need to be edited is important.

If the name is declared in the library then presumably the developer already has control over the library. There are no hide or show clauses for part directives, so finding the specific part directive doesn't seem helpful. Also, it might be in the library's namespace because it's from the parent part (the part of directive) rather than an included part.

So, what's the motivation for this feature?

@FMorschel
Copy link
Contributor Author

FMorschel commented Dec 12, 2024

I feel like this tool would feel more like "if I can't find the declaration inside this file, which direcitve is adding it to this scope?".

If it's being added by the parent file or one of its other parts, we could point to the part of directive.

I feel like this could be a good tool for:

@zoechi wrote on #32234:

I almost always use show. I think it makes code much easier to understand when it shows where certain identifiers come from. Especially UI code often has a lot of components/widget imports and imports for other stuff.
I'm in favor of making this more convenient.
(From #32234 (comment))

But as you said, on part of and part directives you can't add shows/hides so this is harder to follow.

P.S.: Not quite sure about augmentations since I've not tested them and I'm unfamiliar if this could be a way to visualise it but maybe.

@DanTup
Copy link
Collaborator

DanTup commented Dec 12, 2024

I believe the goal for 'Go to Import' is to answer the question "where is this name imported from".
A developer presumably wants to know that in order to control the names that are in scope (via hide and show)

My thought was that if a user has invoked this on something that isn't coming from imports, they are probably still wondering where it's coming from and showing the part (or part of) directive would answer that. If we show nothing in this case, they may still be wondering why it's in-scope and have to manually review multiple levels of parts.

I don't know how big the trees of parts will typically be (presume they will be used more with enhanced parts), though I also don't think we have many features that help understand/visualise that heirarchy.

@bwilkerson
Copy link
Member

... they are probably still wondering where it's coming from ...

Maybe. If I were looking at a library I wasn't familiar with and it was spread across a lot of files I suspect that I'd just navigate to the declaration site rather than looking for which part it's coming from. But I'm not always representative of our users, so maybe.

... showing the part (or part of) directive would answer that.

I'm not sure that it does, so I'm going to push back a bit. If it says that C comes from part 'a.dart';, but only because a.dart includes b.dart, which in turn includes c.dart where the declaration is, did the user actually learn anything useful? If the user navigates to a.dart and can't find C are they likely to be confused or do they have all the information they need at that point? What if there's no reference to C in a.dart so there's no way to find which part directive in a.dart to follow to get closer?

Would a better solution be to (somehow) display the path through the part structure?

Do we need to find a way to display the whole part tree (I mistyped earlier when I said 'graph'), maybe with some indication of where the part in the active editor is in the representation?

@DanTup
Copy link
Collaborator

DanTup commented Dec 12, 2024

Maybe. If I were looking at a library I wasn't familiar with and it was spread across a lot of files I suspect that I'd just navigate to the declaration site rather than looking for which part it's coming from.

Yeah, that's fair. At first I thought "what if the declaration is in some unfamiliar file and I don't know how it's part of this library?".. however it occurs to me now that because part has to match a part of and therefore you could trivially walk back up to the original file (at least I think... it's not possible to get names sideways in parts - only from parents/children?).

If it says that C comes from part 'a.dart';, but only because a.dart includes b.dart, which in turn includes c.dart where the declaration is, did the user actually learn anything useful?

Perhaps not. If they recognised the "rest" of the path to C, maybe learning the first hop would help - but on reflection, I agree maybe it was less useful than I originally thought. It may be useful in the case where it's in the immediate part, but then Go to Definintion would've probably been good enough anyway.

Would a better solution be to (somehow) display the path through the part structure?

I think it probably would. We might have limited ways in which we could show this in VS Code, but I wonder if the hover would be a fair place for it. It's probably similar to what was asked for at #56935, though I think for imports there could be many paths to the declaration, wereas I think with parts it would be a single path (but it could be in either direction)?

(there is also a breadcrumb in VS Code, but it's automatically built from the Document Symbol data and not something we can add arbitrary nodes to)

@FMorschel
Copy link
Contributor Author

If it says that C comes from part 'a.dart';, but only because a.dart includes b.dart, which in turn includes c.dart where the declaration is, did the user actually learn anything useful? If the user navigates to a.dart and can't find C are they likely to be confused or do they have all the information they need at that point? What if there's no reference to C in a.dart so there's no way to find which part directive in a.dart to follow to get closer?

For this I could say the same about exports. They won't actually help me get where is this coming from entirely. For that I'd use the Go to Declaration (CTRL + CLICK).

Would a better solution be to (somehow) display the path through the part structure?

Do we need to find a way to display the whole part tree (I mistyped earlier when I said 'graph'), maybe with some indication of where the part in the active editor is in the representation?

It would help to visualise the structure yes (I would like such a tool) but it wouldn't say whether a declaration is "above" or "below" the current part.

@FMorschel
Copy link
Contributor Author

at least I think... it's not possible to get names sideways in parts - only from parents/children?

You can. The same way you do today if a source file has multiple parts. All of them have access to anything from any other. And now since you can have any of them with inner children, any of their children's declaration would be accessible to you anywhere.

@bwilkerson
Copy link
Member

I think for imports there could be many paths to the declaration, wereas I think with parts it would be a single path (but it could be in either direction)?

It could even be up and down, but yes, there's a single path.

For this I could say the same about exports. They won't actually help me get where is this coming from entirely.

If you meant 'imports' rather than 'exports', I agree. But they will get you to where you can control what's being imported. Getting to a part directive doesn't appear to be useful for anything.

... but it wouldn't say whether a declaration is "above" or "below" the current part.

I'm not sure that concept makes sense, but if it did what would that information tell you? Or, more to the point, what task would that information help you perform?

All of them have access to anything from any other.

Correct. The one new thing in terms of namespaces is that you have a little more control over which imported names are in scope.

@FMorschel
Copy link
Contributor Author

... but it wouldn't say whether a declaration is "above" or "below" the current part.

I'm not sure that concept makes sense, but if it did what would that information tell you? Or, more to the point, what task would that information help you perform?

The point in that phrase was declaration.

It would help me better understand the library structure. For refactorings and similar I think it would be a good tool (even more so if that tree tool existed).

Say some class C is declared in c.dart:

- main.dart
     - f1.dart
         - c.dart
     - f2.dart
         - a.dart
         - b.dart
             - b1.dart

If I'm in f2.dart for example I'd know that it comes from either a direct declaration on main.dart or another part file inside it. In the future it may be easier for me to find c.dart if I change the structure of the library since now I'm using it inside f2.dart (and probably before it was only used in f1.dart). Maybe even move C outside the library and import it or something.

Similar if it was declared inside b1.dart. I would not find the declaration directly on b.dart file so I may rethink about the structure of the part tree I'm creating.

The one new thing in terms of namespaces is that you have a little more control over which imported names are in scope.

It would help me with this.

I know I may not find the declaration directly and that would be my entire point here. "Why isn't this closer?" If it is, good, then were done.


For this I could say the same about exports. They won't actually help me get where is this coming from entirely.

If you meant 'imports' rather than 'exports', I agree. But they will get you to where you can control what's being imported. Getting to a part directive doesn't appear to be useful for anything.

I meant exports in the sense where the declaration is being exported one or many times until it gets to the file being imported.

@bwilkerson
Copy link
Member

I meant exports in the sense where the declaration is being exported one or many times until it gets to the file being imported.

Ok, I see your point, and I agree. Following the chain of exports to try to see where a declaration is coming from is equally difficult.

@scheglov scheglov added the P4 label Dec 14, 2024
@FMorschel
Copy link
Contributor Author

I'm unsure if we'll ever do this. Other things I just wanted to know if you agree a bit on before opening an issue:


Do we need to find a way to display the whole part tree (I mistyped earlier when I said 'graph'), maybe with some indication of where the part in the active editor is in the representation?

I think something like Show type hierarchy from VS Code would be cool for this, do we have an issue?


Similar or integrated to the above, I think a tree of declarations would be nice too.

It could show all top-level declarations and an open/close if they have inner declarations (classes -> methods -> local variables), if you agree this could be imagined to display inside or alongside the above. This way we could see where some declaration is inside a whole library. If we have a similar issue, please let me know.

@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. P4 type-design
Projects
None yet
Development

No branches or pull requests

4 participants