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

fix(publish): make include and exclude work #22720

Merged
merged 14 commits into from
Mar 8, 2024
Merged
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ winres.workspace = true
[dependencies]
deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "proposal", "react", "sourcemap", "transforms", "typescript", "view", "visit"] }
deno_cache_dir = { workspace = true }
deno_config = "=0.12.0"
deno_config = "=0.14.1"
deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
deno_doc = { version = "=0.113.1", features = ["html"] }
deno_emit = "=0.38.2"
Expand Down
14 changes: 7 additions & 7 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,7 @@ impl CliOptions {
pub fn resolve_config_excludes(&self) -> Result<PathOrPatternSet, AnyError> {
let maybe_config_files = if let Some(config_file) = &self.maybe_config_file
{
config_file.to_files_config()?
Some(config_file.to_files_config()?)
} else {
None
};
Expand Down Expand Up @@ -1750,14 +1750,14 @@ fn resolve_files(
if let Some(file_flags) = maybe_file_flags {
if !file_flags.include.is_empty() {
maybe_files_config.include =
Some(PathOrPatternSet::from_relative_path_or_patterns(
Some(PathOrPatternSet::from_include_relative_path_or_patterns(
initial_cwd,
&file_flags.include,
)?);
}
if !file_flags.ignore.is_empty() {
maybe_files_config.exclude =
PathOrPatternSet::from_relative_path_or_patterns(
PathOrPatternSet::from_exclude_relative_path_or_patterns(
initial_cwd,
&file_flags.ignore,
)?;
Expand Down Expand Up @@ -1886,7 +1886,7 @@ mod test {
temp_dir.write("pages/[id].ts", "");

let temp_dir_path = temp_dir.path().as_path();
let error = PathOrPatternSet::from_relative_path_or_patterns(
let error = PathOrPatternSet::from_include_relative_path_or_patterns(
temp_dir_path,
&["data/**********.ts".to_string()],
)
Expand All @@ -1897,7 +1897,7 @@ mod test {
Some(FilePatterns {
base: temp_dir_path.to_path_buf(),
include: Some(
PathOrPatternSet::from_relative_path_or_patterns(
PathOrPatternSet::from_include_relative_path_or_patterns(
temp_dir_path,
&[
"data/test1.?s".to_string(),
Expand All @@ -1908,7 +1908,7 @@ mod test {
)
.unwrap(),
),
exclude: PathOrPatternSet::from_relative_path_or_patterns(
exclude: PathOrPatternSet::from_exclude_relative_path_or_patterns(
temp_dir_path,
&["nested/**/*bazz.ts".to_string()],
)
Expand All @@ -1919,7 +1919,7 @@ mod test {
)
.unwrap();

let mut files = FileCollector::new(|_, _| true)
let mut files = FileCollector::new(|_| true)
.ignore_git_folder()
.ignore_node_modules()
.ignore_vendor_folder()
Expand Down
13 changes: 10 additions & 3 deletions cli/lsp/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1083,7 +1083,7 @@ impl Config {
pub fn get_disabled_paths(&self) -> PathOrPatternSet {
let mut path_or_patterns = vec![];
if let Some(cf) = self.maybe_config_file() {
if let Some(files) = cf.to_files_config().ok().flatten() {
if let Ok(files) = cf.to_files_config() {
for path in files.exclude.into_path_or_patterns() {
path_or_patterns.push(path);
}
Expand All @@ -1095,7 +1095,14 @@ impl Config {
continue;
};
let settings = self.workspace_settings_for_specifier(workspace_uri);
if settings.enable.unwrap_or_else(|| self.has_config_file()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@nayeemrmn this previous condition seemed slightly wrong and caused some trouble in the tests. Is the new condition ok?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah good find, but if enable_paths is specified then it should completely override enable.

if settings.enable_paths.as_ref().map(|p| !p.is_empty()).unwrap_or_else(|| previous_condition) { ... }

let is_enabled = settings
.enable_paths
.as_ref()
.map(|p| !p.is_empty())
.unwrap_or_else(|| {
settings.enable.unwrap_or_else(|| self.has_config_file())
});
if is_enabled {
for path in &settings.disable_paths {
path_or_patterns.push(PathOrPattern::Path(workspace_path.join(path)));
}
Expand Down Expand Up @@ -1177,7 +1184,7 @@ fn specifier_enabled(
workspace_folders: &[(Url, lsp::WorkspaceFolder)],
) -> bool {
if let Some(cf) = config_file {
if let Some(files) = cf.to_files_config().ok().flatten() {
if let Ok(files) = cf.to_files_config() {
if !files.matches_specifier(specifier) {
return false;
}
Expand Down
20 changes: 13 additions & 7 deletions cli/lsp/documents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1341,11 +1341,12 @@ impl Documents {
.inner()
.iter()
.map(|p| match p {
PathOrPattern::Path(p) => {
Cow::Owned(p.to_string_lossy().to_string())
PathOrPattern::Path(p) => p.to_string_lossy(),
PathOrPattern::NegatedPath(p) => {
Cow::Owned(format!("!{}", p.to_string_lossy()))
}
PathOrPattern::RemoteUrl(p) => Cow::Borrowed(p.as_str()),
PathOrPattern::Pattern(p) => Cow::Borrowed(p.as_str()),
PathOrPattern::Pattern(p) => p.as_str(),
})
.collect::<Vec<_>>();
// ensure these are sorted so the hashing is deterministic
Expand Down Expand Up @@ -2061,8 +2062,13 @@ impl Iterator for PreloadDocumentFinder {
if let Ok(entry) = entry {
let path = entry.path();
if let Ok(file_type) = entry.file_type() {
if file_patterns.matches_path(&path) {
if file_type.is_dir() && is_discoverable_dir(&path) {
let is_dir = file_type.is_dir();
let path_kind = match is_dir {
true => deno_config::glob::PathKind::Directory,
false => deno_config::glob::PathKind::File,
};
if file_patterns.matches_path(&path, path_kind) {
if is_dir && is_discoverable_dir(&path) {
self.pending_entries.push_back(PendingEntry::Dir(
path.to_path_buf(),
file_patterns.clone(),
Expand Down Expand Up @@ -2354,7 +2360,7 @@ console.log(b, "hello deno");
file_patterns: FilePatterns {
base: temp_dir.path().to_path_buf(),
include: Some(
PathOrPatternSet::from_relative_path_or_patterns(
PathOrPatternSet::from_include_relative_path_or_patterns(
temp_dir.path().as_path(),
&[
"root1".to_string(),
Expand Down Expand Up @@ -2415,7 +2421,7 @@ console.log(b, "hello deno");
file_patterns: FilePatterns {
base: temp_dir.path().to_path_buf(),
include: Default::default(),
exclude: PathOrPatternSet::from_relative_path_or_patterns(
exclude: PathOrPatternSet::from_exclude_relative_path_or_patterns(
temp_dir.path().as_path(),
&[
"root1".to_string(),
Expand Down
27 changes: 9 additions & 18 deletions cli/tools/bench/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ use crate::tools::test::format_test_error;
use crate::tools::test::TestFilter;
use crate::util::file_watcher;
use crate::util::fs::collect_specifiers;
use crate::util::fs::WalkEntry;
use crate::util::path::is_script_ext;
use crate::util::path::matches_pattern_or_exact_path;
use crate::version::get_user_agent;
use crate::worker::CliMainWorkerFactory;

use deno_config::glob::FilePatterns;
use deno_config::glob::PathOrPattern;
use deno_core::error::generic_error;
use deno_core::error::AnyError;
use deno_core::error::JsError;
Expand Down Expand Up @@ -394,25 +394,16 @@ async fn bench_specifiers(
}

/// Checks if the path has a basename and extension Deno supports for benches.
fn is_supported_bench_path(path: &Path, patterns: &FilePatterns) -> bool {
if !is_script_ext(path) {
fn is_supported_bench_path(entry: WalkEntry) -> bool {
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved
if !is_script_ext(entry.path) {
false
} else if has_supported_bench_path_name(path) {
} else if has_supported_bench_path_name(entry.path) {
true
} else {
} else if let Some(include) = &entry.patterns.include {
// allow someone to explicitly specify a path
let matches_exact_path_or_pattern = patterns
.include
.as_ref()
.map(|p| {
p.inner().iter().any(|p| match p {
PathOrPattern::Path(p) => p == path,
PathOrPattern::RemoteUrl(_) => true,
PathOrPattern::Pattern(p) => p.matches_path(path),
})
})
.unwrap_or(false);
matches_exact_path_or_pattern
matches_pattern_or_exact_path(include, entry.path)
} else {
false
}
}

Expand Down
11 changes: 4 additions & 7 deletions cli/tools/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,23 +388,20 @@ fn collect_coverages(
initial_cwd.to_path_buf(),
)])
} else {
PathOrPatternSet::from_relative_path_or_patterns(
PathOrPatternSet::from_include_relative_path_or_patterns(
initial_cwd,
&files.include,
)?
}
}),
exclude: PathOrPatternSet::from_relative_path_or_patterns(
exclude: PathOrPatternSet::from_exclude_relative_path_or_patterns(
initial_cwd,
&files.ignore,
)
.context("Invalid ignore pattern.")?,
};
let file_paths = FileCollector::new(|file_path, _| {
file_path
.extension()
.map(|ext| ext == "json")
.unwrap_or(false)
let file_paths = FileCollector::new(|e| {
e.path.extension().map(|ext| ext == "json").unwrap_or(false)
})
.ignore_git_folder()
.ignore_node_modules()
Expand Down
12 changes: 7 additions & 5 deletions cli/tools/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,15 @@ pub async fn doc(flags: Flags, doc_flags: DocFlags) -> Result<(), AnyError> {
let module_specifiers = collect_specifiers(
FilePatterns {
base: cli_options.initial_cwd().to_path_buf(),
include: Some(PathOrPatternSet::from_relative_path_or_patterns(
cli_options.initial_cwd(),
source_files,
)?),
include: Some(
PathOrPatternSet::from_include_relative_path_or_patterns(
cli_options.initial_cwd(),
source_files,
)?,
),
exclude: Default::default(),
},
|_, _| true,
|_| true,
)?;
let graph = module_graph_creator
.create_graph(GraphKind::TypesOnly, module_specifiers.clone())
Expand Down
2 changes: 1 addition & 1 deletion cli/tools/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ async fn format_files(
}

fn collect_fmt_files(files: FilePatterns) -> Result<Vec<PathBuf>, AnyError> {
FileCollector::new(|path, _| is_supported_ext_fmt(path))
FileCollector::new(|e| is_supported_ext_fmt(e.path))
.ignore_git_folder()
.ignore_node_modules()
.ignore_vendor_folder()
Expand Down
2 changes: 1 addition & 1 deletion cli/tools/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ async fn lint_files(
}

fn collect_lint_files(files: FilePatterns) -> Result<Vec<PathBuf>, AnyError> {
FileCollector::new(|path, _| is_script_ext(path))
FileCollector::new(|e| is_script_ext(e.path))
.ignore_git_folder()
.ignore_node_modules()
.ignore_vendor_folder()
Expand Down
Loading