Improved project folder support #2692

saul · 2017-03-24T16:54:29Z

Fixes #2178, #2048, #2539, #1095 and #732

Folder node selected

Add Above
- ✨ Add New: added support
- ✨ Add Existing: added support
- ✨ Add Folder added support
Add Below
- ✨ Add New: added support
- ✨ Add Existing: added support
- ✨ Add Folder added support
✨ Add Above/Add Below on an empty folder

File node selected

Add Above
- Add New
  - 🐞 Within sub-folder: fixed
  - At root: already supported
- Add Existing
  - 🐞 Within sub-folder: fixed
  - At root: already supported
Add Below
- Add New
  - 🐞 Within sub-folder: fixed
  - At root: already supported
- Add Existing
  - 🐞 Within sub-folder: fixed
  - At root: already supported

Cut & Paste

🐞 Fix null reference exceptions (Ctrl-X/Ctrl-V causing NullReferenceException, load errors on next start, invalid file reference compile order in *.fsproj #2048) fixed
🐞 Ctrl+X, Ctrl+V adds "Copy ..." prefix to filename fixed
🐞 Solution Explorer/fsproj order is mismatched on paste fixed

Add > Add Existing...

🐞 Add > Add Existing... on the project node should place the file in the correct subfolder fixed
✨ Add > Add Existing... anywhere in the project tree should place the file in the correct subfolder added
✨ Add > Add Existing... should create any necessary intermediate directories added
🐞 Add > Add Existing... now works with files that are outside of the project hierarchy fixed

"Add Folder"

✨ Add Above/Below > New Folder menu item added
✨ Enable "Add Folder" on project node added

Miscellaneous

🐞 Better support for non-Windows style directory separators in .fsproj fixed
✨ Add Above/Below > Add Existing... should fail early if the file is in a different subtree added
🔬 Test multiple folders with the same name
🔬 Fix all bugs with linked files
✨ If the folder exists on disk, use it instead of throwing an error
🐞 Deleting a folder with links to files elsewhere on disk is deleting those files

Fixes dotnet#2178

vasily-kirichenko · 2017-03-24T17:28:13Z

This is great.

No "Add folder" menu

saul · 2017-03-24T17:29:23Z

@vasily-kirichenko is that a feature request or a regression? :)

vasily-kirichenko · 2017-03-24T17:34:29Z

That was a feature request :)

"Add new item" into a folder adds it to the end in the project file:

vasily-kirichenko · 2017-03-24T17:35:34Z

...then I tried to move File1.fs up:

It's moved in solution explorer, but the project file has not changed, it's same as on the picture above.

saul · 2017-03-24T17:59:18Z

Ah. I know why this is. The path separators need to be backslash

vasily-kirichenko · 2017-03-24T18:01:47Z

So... I think it should work with both types of slashes since it's very likely / is used on linux and mac (if you open a project created in, say, Xamarin on Mac).

dsyme · 2017-03-24T18:13:12Z

Wow, it would be great to get this in.

cloudRoutine · 2017-03-24T19:06:46Z

right now it'll put it in the right folder if you click on the folder itself and do add item

vasily-kirichenko · 2017-03-24T19:08:54Z

@cloudRoutine that's what I did.

cloudRoutine · 2017-03-24T19:11:10Z

bizarre, this is the only way that works consistently for me

saul · 2017-03-24T19:11:20Z

@cloudRoutine it's because Vasily had Linux-style directory separators in some of the files in his .fsproj - I'm fixing that now

saul · 2017-03-24T19:50:27Z

@vasily-kirichenko @cloudRoutine fixed in 4d33380

I've also just re-enabled support for "Add folder" on the project node:

forki · 2017-03-24T19:57:05Z

Wow this is so cool. Thanks so much for doing this.

vasily-kirichenko · 2017-03-24T20:23:33Z

awesome! will test it tomorrow.

Fixes dotnet#2048

saul · 2017-03-24T21:05:32Z

Just fixed cut/copy and pasting files around:

cc @abelbraaksma

… subfolder

cartermp · 2017-03-25T01:21:17Z

@saul You're a rockstar. Love it!

vasily-kirichenko · 2017-03-25T08:48:56Z

@saul everything works great. Very cool :) Any plans to add drag-n-drop support? :)

forki · 2017-03-25T09:44:17Z

Can we try to get the first batch in? It would immediately be helpful. Further improvements like draggedy-droppery-mousy-minkie could be added in another PR.

saul · 2017-03-25T09:45:17Z

@forki my plan is to just fix the stuff that's there for the minute (see my incomplete check boxes in the PR description

saul · 2017-03-25T21:24:23Z

A bit more progress - coming close for this to be merged.

I've reworked how the Solution Explorer and project file are synchronised. Any operation on a file in the Solution Explorer is no longer attempted to be 'replayed' on the project file (e.g adding a file to the Solution Explorer would try to move it to where we expected it in the project file). Instead, the file is added to the Solution Explorer to the most reasonable place, and then moved elsewhere if possible. Wherever the file ends up, the build project will replicate. This means it should no longer be possible for the Solution Explorer and project file to become 'out of sync'.

An example error is shown below:

dsyme · 2017-03-31T14:03:23Z

.... using the #load directive to reference the files. Wouldn't it be possible to relay on this to improve the xplat story and removing the file ordering issues?

@nelak Yes, see the suggestion links above - please add your comments there.

cloudRoutine · 2017-04-03T04:59:46Z

@saul I hope you haven't given up on this PR in favor of CPS work, which is all well and good but it'll be months before we get our hands on it.

Whereas this makes some really useful material gains that can be taken advantage of today (I merge this PR into my custom tools branch, no nightlies for me 😉 )

dsyme · 2017-04-03T10:57:39Z

@cloudRoutine @saul @KevinRansom In the context of open source working, I think there can be "prime the pump" advantages to accepting feature work like this, even if it is later thrown away and replaced when CPS comes.

Specifically, it can help familiarize contributors with a feature area, making it easier to implement/debug/extend/correct/... the feature second time around. And people get to use and debug the UX for the feature early. And people also get very motivated to ensure the feature works in the later version as well.

I think the community experience with VFPT was somewhat similar - although built with the pre-Roslyn VS integration, the lessons, code, UX and so on were all incredibly valuable. And it was/is a highly valuable set of tools in its own right. So VFPT was definitely "worth it" as it primed the pump for the Roslynized version, as well as being useful in its own right.

cartermp · 2017-04-03T15:08:53Z

I agree that this would be good to get in. The CPS work is currently scheduled for 15.3, which will be in a July timeframe, but as that work progresses we may have to cut scope. I'm also not convinced that we'll be able to completely switch over to that project system for .NET Framework projects in that time frame. Since .NET Framework represents the vast majority of projects in .NET, improvements like this are wonderful.

cartermp · 2017-04-03T15:09:00Z

@dotnet-bot test this please

KevinRansom · 2017-04-03T15:59:34Z

@dsyme @saul I want to accept this. I think the CI failures where what was holding me back so far. I am also particularly keen to see a prototype of the compilation order node. Since that offers a solution to our long existing issue with file ordering and folders.

Kevin

saul · 2017-04-03T19:20:23Z

@cloudRoutine @KevinRansom I don't think this will be completed this week. Time constraints are making this week particularly difficult, but next week I'm hoping to have this (and the rest of my PRs!) merged.

Apologies for the delay, but rest assured I haven't given up on this.

saul · 2017-04-15T16:09:20Z

I think the following tasks should be moved to a separate issue/PR:

If the folder doesn't exist, create a virtual folder
If you add a concrete item to a virtual folder create a folder on disk and add the item
If you add a linked item to a virtual folder it stays virtual

I think they're great things to have but I think this is feature creep. 'Virtual folders' don't exist as a concept within the project system at present, so it'd be a lot of new code to add this.

Also will take a look at the failing tests soon then this will be ready to merge.

saul · 2017-04-15T16:11:58Z

Also @cloudRoutine as requested, here's what happens when you try to rename a folder (that is not empty) to a folder that already exists:

Note if the folder you're trying to rename is empty, it does it without complaining now.

cloudRoutine · 2017-04-16T07:10:20Z

I think they're great things to have but I think this is feature creep. 'Virtual folders' don't exist as a concept within the project system at present, so it'd be a lot of new code to add this.

I don't know what you mean by "exist as a concept" but you can definitely have virtual folders within a project.

  <ItemGroup>
    <Compile Include="AssemblyInfo.fs" />
    <Compile Include="Library1.fs" />
    <None Include="Script.fsx" />
    <Content Include="packages.config" />
    <Compile Include="one.fs">
      <Link>virtual-folder\one.fs</Link>
    </Compile>
    <Compile Include="two.fs">
      <Link>virtual-folder\two.fs</Link>
    </Compile>
  </ItemGroup>

You see a folder node in the project tree, there is no corresponding folder on disk.
All I want is that if I add another linked item to that "virtual folder" there shouldn't be a new folder created on disk.

@saul whatever you're thinking of is way more complicated than it needs to be

saul · 2017-04-19T16:59:16Z

No idea what the test failure is here because I can't even view the whole file

majocha · 2017-04-19T17:04:35Z

@saul

Errors and Failures

1) Failed : Tests.ProjectSystem.ProjectItems.AddNewItem.ItemAppearsAtBottomOfFsprojFileEvenIfUnknownItemWithSameName
Expected
["orig.fs"; "a.fs"]
but got
["a.fs"; "orig.fs"]

saul · 2017-04-19T17:05:57Z

@majocha Thanks - but how in god's name did you find that?

majocha · 2017-04-19T17:06:53Z

@saul I saved `view as plain text' link to text file :)

charlesroddie · 2017-04-19T17:47:56Z

@saul
The screenshots here look great!
But please don't make the solution order different from compile order.

…at has the same name as an existing file

saul · 2017-04-19T20:21:19Z

Removed the failing test as it's impossible to reproduce in a real-world scenario. The test was doing the following:

In a project with the following MSBuild:

                <ItemGroup>
                    <Unknown Include="a.fs" />
                    <Compile Include="orig.fs" />
                </ItemGroup>

It was simulating adding the "a.fs" to the project. Now in VS proper this is a no-op - VS detects that the file is already in the project and does nothing. The test was simulating this about 5 layers deep and so some assumptions were failing. I don't believe they need addressing as the scenario can't be reproduced in a real-life scenario anyway.

KevinRansom · 2017-04-20T15:25:31Z

@saul is it your recommendation that this is reviewable.

saul · 2017-04-20T16:38:37Z

@KevinRansom reviewable and in a state to be merged

KevinRansom

nice work.

KevinRansom · 2017-04-20T18:42:47Z

@saul Thanks for this it looks great.

Kevin

Significantly improved project folder support

240efe2

Fixes dotnet#2178

msftclas added the cla-already-signed label Mar 24, 2017

saul added 2 commits March 24, 2017 19:34

Better support for non-Windows style directory separators in .fsproj

4d33380

Enable support for "Add folder" on project node

3f964b8

Fix copy/pasting files within subfolders

1757503

Fixes dotnet#2048

"Add existing" on the project node now places the file in the correct…

1c524d2

… subfolder

saul added 2 commits March 25, 2017 10:51

"Add existing..." now creates any necessary intermediate directories

fa57fb9

Improved resilience of Add Above/Add Below with folders

9b505d0

Localised new error messages

0548af8

cloudRoutine mentioned this pull request Mar 26, 2017

Feature request for VS 2017: Folders in F# projects #2539

Closed

saul added 3 commits April 15, 2017 11:57

Merge branch 'master' into project-folder-fixes

bfb0657

If the folder exists on disk use that instead

7772800

Do not delete linked files when deleting folders

92984a3

saul changed the title ~~[WIP] Improved project folder support~~ Improved project folder support Apr 19, 2017

saul added 2 commits April 19, 2017 19:03

Merge remote-tracking branch 'upstream/master' into project-folder-fixes

faa3a7f

Remove bogus test - it's not possible to add a file to the project th…

eed0690

…at has the same name as an existing file

KevinRansom approved these changes Apr 20, 2017

View reviewed changes

KevinRansom merged commit dd18b3e into dotnet:master Apr 20, 2017

This was referenced Apr 20, 2017

Add support for creating and placing files in folders #1095

Closed

Cutting and pasting an .fsx file in Solution Explorer errors #732

Closed

cartermp mentioned this pull request Dec 14, 2017

Fix inability to drag files inside folders #3550 #4095

Merged

Improved project folder support #2692

Improved project folder support #2692

Conversation

saul commented Mar 24, 2017 • edited Loading

Folder node selected

File node selected

Cut & Paste

Add > Add Existing...

"Add Folder"

Miscellaneous

vasily-kirichenko commented Mar 24, 2017

saul commented Mar 24, 2017

vasily-kirichenko commented Mar 24, 2017

vasily-kirichenko commented Mar 24, 2017 • edited Loading

saul commented Mar 24, 2017

vasily-kirichenko commented Mar 24, 2017

dsyme commented Mar 24, 2017

cloudRoutine commented Mar 24, 2017

vasily-kirichenko commented Mar 24, 2017

cloudRoutine commented Mar 24, 2017

saul commented Mar 24, 2017

saul commented Mar 24, 2017 • edited Loading

forki commented Mar 24, 2017

vasily-kirichenko commented Mar 24, 2017

saul commented Mar 24, 2017

cartermp commented Mar 25, 2017

vasily-kirichenko commented Mar 25, 2017

forki commented Mar 25, 2017

saul commented Mar 25, 2017

saul commented Mar 25, 2017

dsyme commented Mar 31, 2017

cloudRoutine commented Apr 3, 2017

dsyme commented Apr 3, 2017

cartermp commented Apr 3, 2017

cartermp commented Apr 3, 2017

KevinRansom commented Apr 3, 2017

saul commented Apr 3, 2017

saul commented Apr 15, 2017

saul commented Apr 15, 2017

cloudRoutine commented Apr 16, 2017

saul commented Apr 19, 2017 • edited Loading

majocha commented Apr 19, 2017

saul commented Apr 19, 2017

majocha commented Apr 19, 2017

charlesroddie commented Apr 19, 2017

saul commented Apr 19, 2017

KevinRansom commented Apr 20, 2017

saul commented Apr 20, 2017

KevinRansom left a comment

Choose a reason for hiding this comment

KevinRansom commented Apr 20, 2017

saul commented Mar 24, 2017 •

edited

Loading

vasily-kirichenko commented Mar 24, 2017 •

edited

Loading

saul commented Mar 24, 2017 •

edited

Loading

saul commented Apr 19, 2017 •

edited

Loading