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

Handle folders in source-generated file paths #68494

Merged
merged 3 commits into from
Jun 15, 2023

Conversation

jjonescz
Copy link
Contributor

@jjonescz jjonescz commented Jun 8, 2023

Fixes #68489.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 8, 2023
@jjonescz jjonescz marked this pull request as ready for review June 8, 2023 17:57
@jjonescz jjonescz requested a review from a team as a code owner June 8, 2023 17:57

await TestServices.Editor.PlaceCaretAsync(HelloWorldGenerator.GeneratedFolderClassName, charsOffset: 0, HangMitigatingCancellationToken);
await TestServices.Editor.GoToDefinitionAsync(HangMitigatingCancellationToken);
Assert.Equal($"{HelloWorldGenerator.GeneratedFolderName}/{HelloWorldGenerator.GeneratedFolderClassName}.cs {ServicesVSResources.generated_suffix}", await TestServices.Shell.GetActiveWindowCaptionAsync(HangMitigatingCancellationToken));
Copy link
Member

Choose a reason for hiding this comment

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

This would seem a bit strange to me to have the relative path in the caption like this since that's not something we do in other parts of VS. Should we be setting the caption to be just the final portion of the path here?

Copy link
Contributor Author

@jjonescz jjonescz Jun 9, 2023

Choose a reason for hiding this comment

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

It's just the full hint name which seems fine to me. It looks like this (note that it's also displayed like that in the solution explorer, something that can be improved in the future - to be instead displayed as a directory tree):

image

Copy link
Member

Choose a reason for hiding this comment

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

I guess this is better than it being broken, but not sure if this will break things...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can change the title to only include filename if you prefer, but I don't know how to make the solution explorer to display folder tree. I hope this is fine for now and can be improved in a follow up PR if needed.

Copy link
Member

Choose a reason for hiding this comment

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

My concern mostly is if something or somebody will break seeing slashes in the tab name, but if something breaks we'll probably discover it soon enough. The solution explorer doing a folder tree would indeed require some significant rework of that area.

Copy link
Member

Choose a reason for hiding this comment

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

I haven't looked closely at this pr , but I assume that this is similar to the workspace apis, and one can pass an array of folder names along?

If that's the case @jasonmalinowski , I would assume we could represent this pretty easily in the workspace and present it nicely.

Copy link
Member

Choose a reason for hiding this comment

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

@CyrusNajmabadi The workspace representation isn't the problem here but simply how the APIs work for adding things to solution explorer. Maybe it's not "significant" work, as much as "slightly irritating" since the model involves a bunch of MEF and things having to return nodes one level at a time.


await TestServices.Editor.PlaceCaretAsync(HelloWorldGenerator.GeneratedFolderClassName, charsOffset: 0, HangMitigatingCancellationToken);
await TestServices.Editor.GoToDefinitionAsync(HangMitigatingCancellationToken);
Assert.Equal($"{HelloWorldGenerator.GeneratedFolderName}/{HelloWorldGenerator.GeneratedFolderClassName}.cs {ServicesVSResources.generated_suffix}", await TestServices.Shell.GetActiveWindowCaptionAsync(HangMitigatingCancellationToken));
Copy link
Member

Choose a reason for hiding this comment

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

I guess this is better than it being broken, but not sure if this will break things...

@jjonescz jjonescz added the New Feature - Source Generators Source Generators label Jun 15, 2023
@jjonescz jjonescz merged commit 7a7690e into dotnet:main Jun 15, 2023
24 checks passed
@jjonescz jjonescz deleted the 68489-SG-Dirs-IDE branch June 15, 2023 08:40
@ghost ghost added this to the Next milestone Jun 15, 2023
@RikkiGibson RikkiGibson modified the milestones: Next, 17.7 P3 Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE New Feature - Source Generators Source Generators untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Source Generator files with directories are not displayed correctly in IDE
5 participants