-
Notifications
You must be signed in to change notification settings - Fork 75
Fix excluded files and folder deletion triggers #112
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using Microsoft.DotNet.ProjectModel.Files; | ||
|
||
namespace Microsoft.DotNet.Watcher.Core | ||
{ | ||
internal static class IncludeContextExtensions | ||
{ | ||
public static IEnumerable<string> ResolveFiles(this IncludeContext context) | ||
{ | ||
if (context == null) | ||
{ | ||
throw new ArgumentNullException(nameof(context)); | ||
} | ||
|
||
return IncludeFilesResolver | ||
.GetIncludeFiles(context, "/", diagnostics: null) | ||
.Select(f => f.SourcePath); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,11 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.IO; | ||
using System.Linq; | ||
using Microsoft.DotNet.ProjectModel.Files; | ||
using Microsoft.DotNet.ProjectModel.Graph; | ||
|
||
namespace Microsoft.DotNet.Watcher.Core.Internal | ||
|
@@ -15,15 +17,34 @@ public Project(ProjectModel.Project runtimeProject) | |
ProjectFile = runtimeProject.ProjectFilePath; | ||
ProjectDirectory = runtimeProject.ProjectDirectory; | ||
|
||
Files = runtimeProject.Files.SourceFiles.Concat( | ||
runtimeProject.Files.ResourceFiles.Values.Concat( | ||
runtimeProject.Files.PreprocessSourceFiles.Concat( | ||
runtimeProject.Files.SharedFiles))).Concat( | ||
new string[] { runtimeProject.ProjectFilePath }) | ||
.ToList(); | ||
var compilerOptions = runtimeProject.GetCompilerOptions(targetFramework: null, configurationName: null); | ||
|
||
var filesToWatch = new List<string>() { runtimeProject.ProjectFilePath }; | ||
if (compilerOptions?.CompileInclude != null) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry if my reply was confusing before, I meant that |
||
{ | ||
filesToWatch.AddRange(compilerOptions.CompileInclude.ResolveFiles()); | ||
} | ||
else | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this be additive? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my questions for @ajaybhargavb |
||
{ | ||
filesToWatch.AddRange(runtimeProject.Files.SourceFiles); | ||
} | ||
|
||
if (compilerOptions?.EmbedInclude != null) | ||
{ | ||
filesToWatch.AddRange(compilerOptions.EmbedInclude.ResolveFiles()); | ||
} | ||
else | ||
{ | ||
filesToWatch.AddRange(runtimeProject.Files.ResourceFiles.Values); | ||
} | ||
|
||
filesToWatch.AddRange(runtimeProject.Files.SharedFiles); | ||
filesToWatch.AddRange(runtimeProject.Files.PreprocessSourceFiles); | ||
|
||
Files = filesToWatch; | ||
|
||
var projectLockJsonPath = Path.Combine(runtimeProject.ProjectDirectory, "project.lock.json"); | ||
|
||
if (File.Exists(projectLockJsonPath)) | ||
{ | ||
var lockFile = LockFileReader.Read(projectLockJsonPath, designTime: false); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,6 +107,10 @@ private void GetProjectFilesClosure(string projectFile, ISet<string> closure) | |
foreach (var file in project.Files) | ||
{ | ||
closure.Add(file); | ||
|
||
// We need to add the folder because folder deletions only trigger | ||
// for the folder, not for the files inside it | ||
closure.Add(Path.GetDirectoryName(file)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you checked if watching the folder causes any triggers to be fired for any changes to the files underneath it, including the excluded files? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, that's a good point. Let me check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems to work |
||
} | ||
|
||
foreach (var dependency in project.ProjectDependencies) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you have files included/excluded per configuration/framework?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ajaybhargavb this is a question for you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compile
andembed
are part ofbuildOptions
which can be framework/configuration specific. So yes the schema supports it. But I am not sure if it makes sense to include/exclude files for specific framework/configuration.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are rare scenarios like that but I am not sure if we need to proactively address them now.