[Core] Avoid showing duplicated files in project tree #9309
Conversation
|
@monojenkins backport release-8.4 |
main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui.Pads.ProjectPad/FolderNodeBuilder.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
There might be a big perf hit from object reference equality vs full string match
There was a problem hiding this comment.
Changed to have FilePath as the key, but not sure what to do with the perf, the work around works because we check the file paths, as the files are duplicated in project.Files, so can't do object reference equality.
There are some cases where, it seems, we don't evaluate conditions as we should, resulting in duplicated items being added to the project and, thus, also shown in the project tree (see dotnet/aspnetcore#17088). Also, by just adding a <None Include="*"/>, duplicated files show also. So, due to this limitation on the project system, work around it on the project tree, by checking for duplicates based on the files' paths. Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1005277
rodrmoya
force-pushed
the
pr-fix-dupe-evaluated-items
branch
from
05eb293 to
24ba1ff
Compare
|
@monojenkins backport release-8.4 |
| { | ||
| string folderPrefix = folder + Path.DirectorySeparatorChar; | ||
|
|
||
| var visitedFiles = new HashSet<FilePath> (); |
There was a problem hiding this comment.
We still need a comment on the fact that this hashset is needed as a workaround.
| } | ||
|
|
||
| readonly ConcurrentDictionary<ProjectFile, ProjectFileNestingInfo> projectFiles = new ConcurrentDictionary<ProjectFile, ProjectFileNestingInfo> (); | ||
| readonly ConcurrentDictionary<FilePath, ProjectFileNestingInfo> projectFiles = new ConcurrentDictionary<FilePath, ProjectFileNestingInfo> (); |
There was a problem hiding this comment.
Add a comment that the FilePath is used as a workaround, we should be using ProjectFile.
|
Alternative fix here which should address the underlying problem of Conditions being ignored on the Content items - #9407 |
|
Closing this one then |
|
|
||
| return projectFiles.TryGetValue (inputFile, out var nestingInfo) ? nestingInfo.Children : null; | ||
| return projectFiles.TryGetValue (inputFile.FilePath, out var nestingInfo) ? nestingInfo.Children : null; | ||
| } |
There was a problem hiding this comment.
@rodrmoya Can you remember why you had to change the FileNestingService here for the duplicate files? Just wondering if the other pull request is enough to fix the problem since that change would only affect the files in the Solution pad window, not the files belonging to the project.
There was a problem hiding this comment.
hmm, so FileNestingService deals with the files in the project, not the solution pad really, so if the files in the project are still duplicated, I guess maybe we need this change after all?
There was a problem hiding this comment.
I did not see any duplicate files in the Solution pad. Just wondering what would happen here with the nesting if Project.Files contained duplicate files, but with different conditions.
There was a problem hiding this comment.
Or well, maybe it's not needed at all, as FileNestingService is called from the node builders, so if those don't get the duplicated files, it should be ok. But not sure, as the service listens to FileAdded/Removed events on the project to keep the data up to date
There was a problem hiding this comment.
I think it might be affected indeed, as files are stored on a dictionary -> https://github.com/mono/monodevelop/blob/master/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Projects.FileNesting/FileNestingService.cs#L198 with the ProjectFile being the key, so maybe it makes sense to get the FileNestingService's changes of this PR, although, as @Therzok mentioned, that adds a big perf hit :/
I'll have a look and see how/if it gets affected
There are some cases where, it seems, we don't evaluate conditions as we should,
resulting in duplicated items being added to the project and, thus, also shown
in the project tree (see dotnet/aspnetcore#17088).
Also, by just adding a , duplicated files show also.
So, due to this limitation on the project system, work around it on the project
tree, by checking for duplicates based on the files' paths.
Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1005277