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

Fixed non-script file creating a stand alone project #4307

Closed
wants to merge 1 commit into from

Conversation

TIHan
Copy link
Contributor

@TIHan TIHan commented Feb 2, 2018

Fixes dotnet/project-system#3195 (comment)

When creating a new project, the default file that gets added will create a stand alone project based on itself because the project hasn't been added to the solution yet in SetupNewTextView. We can safely ignore this.

@TIHan TIHan changed the base branch from master to dev15.7 February 2, 2018 18:40
@TIHan TIHan changed the base branch from dev15.7 to master February 2, 2018 18:41
@saul
Copy link
Contributor

saul commented Feb 2, 2018

What happens now if you drag-and-drop an .fs file into VS that isn't part of the project?

@saul
Copy link
Contributor

saul commented Feb 2, 2018

Also worth speaking to @KevinRansom as he added this logic for CPS - this case never used to be here

@KevinRansom
Copy link
Member

@saul, we went over it this morning.

@Pilchie Pilchie requested a review from KevinRansom February 3, 2018 01:34
@Pilchie Pilchie added this to the 15.6 milestone Feb 3, 2018
KevinRansom
KevinRansom previously approved these changes Feb 3, 2018
Copy link
Member

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

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

Nice ...

Thanks for this.

@dsyme
Copy link
Contributor

dsyme commented Feb 3, 2018

What happens now if you drag-and-drop an .fs file into VS that isn't part of the project?

What's the answer to this?

@KevinRansom KevinRansom dismissed their stale review February 4, 2018 05:59

Changed my mind, I don't think this is the right fix.

@KevinRansom
Copy link
Member

@dsyme , @ saul

to answer the copy/paste question: When a file is pasted into a project, the file is added to the project. When the user double clicks on the file, it will open, and be colourised and have intellisense.

@TIHan, @Pilchie, @brettfo, @cartermp

I have given this more thought today, and the original code enabled loose F# files that are not part of the current project or solution to be colourized and intellisensed. With this change they will be opened without colorization and without intellisense, which is a giant step backwards.

There is an underlying problem in the Project System or CPS that is causing the file deletion to try to transfer ownership of a deleted file between projects. That seems a bit wierd right there, why transfer ownership ... just delete it and make sure all of it's containing projects know that seems like the way to go.

Anyway, this change treats the symptom rather than the actual underlying issue, I think we need to delve a bit deeper to find that underlying issue.

Kevin

@TIHan
Copy link
Contributor Author

TIHan commented Feb 4, 2018

Thinking about it more as well, @KevinRansom is right. Will need to investigate more into this.

@saul
Copy link
Contributor

saul commented Feb 4, 2018

That’s a shame - that’s what I thought. I spent some time looking at SetupNewTextView in #1944. The whole thing strikes me as wrong - as does the project setup code (the mailbox processor that runs after solution load). It just seems like the wrong way of setting up projects for individual files.

For CPS at least, can’t we follow the Roslyn convention for standalone files? I know there’s the MiscellaneousFilesWorkspace, but I don’t think that’s used for .cs files that aren’t part of a project (I know it’s used for script files, though).

@KevinRansom KevinRansom closed this Feb 4, 2018
@Pilchie
Copy link
Member

Pilchie commented Feb 4, 2018

Yes, MiscellaneousFilesWorkspace is specifically designed for standalone files in Roslyn.

@dpoeschl
Copy link

dpoeschl commented Feb 5, 2018

@KevinRansom @TIHan

There is an underlying problem in the Project System or CPS that is causing the file deletion to try to transfer ownership of a deleted file between projects. That seems a bit wierd right there, why transfer ownership ... just delete it and make sure all of it's containing projects know that seems like the way to go.

Roslyn does this for linked files (any file in multiple projects with the same file path) because you could be unloading one project or the other, or other weird scenarios like that. Seems like there are two root problems.

  1. Deleting a file that's in multiple projects shouldn't crash. Whether we "just delete it and let its containing projects know" (this change would need lots of testing) or make the existing Roslyn ownership-transfer logic not fail.

  2. For new F# projects specifically, why is the file ending up in multiple projects? Even if we fix 1 above, the Nav Bar will still have two entries in the project dropdown that have the same text and which switch you back & forth between the "file project" & "project project" context, whatever implications that has (Navigation? Semantic classification? Etc.).

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

Successfully merging this pull request may close these issues.

6 participants