Skip to content

Commit

Permalink
Merge pull request #4794 from epage/osstr
Browse files Browse the repository at this point in the history 
perf!(lex): Build faster by removing `os_str_bytes`
  • Loading branch information
@epage
epage authored Mar 27, 2023
2 parents 9712987 + 6419a0d commit 627a94f
Show file tree
Hide file tree
Showing 10 changed files with 432 additions and 164 deletions.
4 changes: 0 additions & 4 deletions Cargo.lock
View file

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

14 changes: 7 additions & 7 deletions clap_builder/src/builder/debug_asserts.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::cmp::Ordering;

use clap_lex::RawOsStr;
use clap_lex::OsStrExt as _;

use crate::builder::OsStr;
use crate::builder::ValueRange;
Expand Down Expand Up @@ -841,16 +841,16 @@ fn assert_defaults<'d>(
for default_os in defaults {
let value_parser = arg.get_value_parser();
let assert_cmd = Command::new("assert");
if let Some(delim) = arg.get_value_delimiter() {
let default_os = RawOsStr::new(default_os);
for part in default_os.split(delim) {
if let Err(err) = value_parser.parse_ref(&assert_cmd, Some(arg), &part.to_os_str())
{
if let Some(val_delim) = arg.get_value_delimiter() {
let mut val_delim_buffer = [0; 4];
let val_delim = val_delim.encode_utf8(&mut val_delim_buffer);
for part in default_os.split(val_delim) {
if let Err(err) = value_parser.parse_ref(&assert_cmd, Some(arg), part) {
panic!(
"Argument `{}`'s {}={:?} failed validation: {}",
arg.get_id(),
field,
part.to_str_lossy(),
part.to_string_lossy(),
err
);
}
Expand Down
56 changes: 20 additions & 36 deletions clap_builder/src/parser/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ use std::{
ffi::{OsStr, OsString},
};

// Third Party
use clap_lex::RawOsStr;
use clap_lex::RawOsString;
use clap_lex::OsStrExt as _;

// Internal
use crate::builder::{Arg, Command};
Expand Down Expand Up @@ -93,9 +91,8 @@ impl<'cmd> Parser<'cmd> {
}

debug!(
"Parser::get_matches_with: Begin parsing '{:?}' ({:?})",
"Parser::get_matches_with: Begin parsing '{:?}'",
arg_os.to_value_os(),
arg_os.to_value_os().as_raw_bytes()
);

// Has the user already passed '--'? Meaning only positional args follow
Expand Down Expand Up @@ -291,7 +288,7 @@ impl<'cmd> Parser<'cmd> {
} else {
let trailing_values = false;
let arg_values = matcher.pending_values_mut(id, None, trailing_values);
arg_values.push(arg_os.to_value_os().to_os_str().into_owned());
arg_values.push(arg_os.to_value_os().to_owned());
if matcher.needs_more_vals(arg) {
ParseResult::Opt(arg.get_id().clone())
} else {
Expand Down Expand Up @@ -411,7 +408,7 @@ impl<'cmd> Parser<'cmd> {
Some(Identifier::Index),
trailing_values,
);
arg_values.push(arg_os.to_value_os().to_os_str().into_owned());
arg_values.push(arg_os.to_value_os().to_owned());
}

// Only increment the positional counter if it doesn't allow multiples
Expand Down Expand Up @@ -548,7 +545,7 @@ impl<'cmd> Parser<'cmd> {
// Checks if the arg matches a subcommand name, or any of its aliases (if defined)
fn possible_subcommand(
&self,
arg: Result<&str, &RawOsStr>,
arg: Result<&str, &OsStr>,
valid_arg_found: bool,
) -> Option<&str> {
debug!("Parser::possible_subcommand: arg={:?}", arg);
Expand Down Expand Up @@ -723,8 +720,8 @@ impl<'cmd> Parser<'cmd> {
fn parse_long_arg(
&mut self,
matcher: &mut ArgMatcher,
long_arg: Result<&str, &RawOsStr>,
long_value: Option<&RawOsStr>,
long_arg: &str,
long_value: Option<&OsStr>,
parse_state: &ParseState,
pos_counter: usize,
valid_arg_found: &mut bool,
Expand All @@ -741,14 +738,6 @@ impl<'cmd> Parser<'cmd> {
}

debug!("Parser::parse_long_arg: Does it contain '='...");
let long_arg = match long_arg {
Ok(long_arg) => long_arg,
Err(long_arg) => {
return Ok(ParseResult::NoMatchingArg {
arg: long_arg.to_str_lossy().into_owned(),
});
}
};
if long_arg.is_empty() {
debug_assert!(
long_value.is_some(),
Expand Down Expand Up @@ -805,7 +794,7 @@ impl<'cmd> Parser<'cmd> {
used.push(arg.get_id().clone());

Ok(ParseResult::UnneededAttachedValue {
rest: rest.to_str_lossy().into_owned(),
rest: rest.to_string_lossy().into_owned(),
used,
arg: arg.to_string(),
})
Expand Down Expand Up @@ -902,7 +891,7 @@ impl<'cmd> Parser<'cmd> {
Ok(c) => c,
Err(rest) => {
return Ok(ParseResult::NoMatchingArg {
arg: format!("-{}", rest.to_str_lossy()),
arg: format!("-{}", rest.to_string_lossy()),
});
}
};
Expand Down Expand Up @@ -938,8 +927,8 @@ impl<'cmd> Parser<'cmd> {
// Cloning the iterator, so we rollback if it isn't there.
let val = short_arg.clone().next_value_os().unwrap_or_default();
debug!(
"Parser::parse_short_arg:iter:{}: val={:?} (bytes), val={:?} (ascii), short_arg={:?}",
c, val, val.as_raw_bytes(), short_arg
"Parser::parse_short_arg:iter:{}: val={:?}, short_arg={:?}",
c, val, short_arg
);
let val = Some(val).filter(|v| !v.is_empty());

Expand All @@ -950,7 +939,7 @@ impl<'cmd> Parser<'cmd> {
//
// e.g. `-xvf`, when require_equals && x.min_vals == 0, we don't
// consume the `vf`, even if it's provided as value.
let (val, has_eq) = if let Some(val) = val.and_then(|v| v.strip_prefix('=')) {
let (val, has_eq) = if let Some(val) = val.and_then(|v| v.strip_prefix("=")) {
(Some(val), true)
} else {
(val, false)
Expand Down Expand Up @@ -991,7 +980,7 @@ impl<'cmd> Parser<'cmd> {
fn parse_opt_value(
&self,
ident: Identifier,
attached_value: Option<&RawOsStr>,
attached_value: Option<&OsStr>,
arg: &Arg,
matcher: &mut ArgMatcher,
has_eq: bool,
Expand Down Expand Up @@ -1032,7 +1021,7 @@ impl<'cmd> Parser<'cmd> {
})
}
} else if let Some(v) = attached_value {
let arg_values = vec![v.to_os_str().into_owned()];
let arg_values = vec![v.to_owned()];
let trailing_idx = None;
let react_result = ok!(self.react(
Some(ident),
Expand All @@ -1054,13 +1043,8 @@ impl<'cmd> Parser<'cmd> {
}
}

fn check_terminator(&self, arg: &Arg, val: &RawOsStr) -> Option<ParseResult> {
if Some(val)
== arg
.terminator
.as_ref()
.map(|s| RawOsStr::from_str(s.as_str()))
{
fn check_terminator(&self, arg: &Arg, val: &OsStr) -> Option<ParseResult> {
if Some(val) == arg.terminator.as_ref().map(|s| OsStr::new(s.as_str())) {
debug!("Parser::check_terminator: terminator={:?}", arg.terminator);
Some(ParseResult::ValuesDone)
} else {
Expand Down Expand Up @@ -1156,17 +1140,17 @@ impl<'cmd> Parser<'cmd> {
if self.cmd.is_dont_delimit_trailing_values_set() && trailing_idx == Some(0) {
// Nothing to do
} else {
let mut val_delim_buffer = [0; 4];
let val_delim = val_delim.encode_utf8(&mut val_delim_buffer);
let mut split_raw_vals = Vec::with_capacity(raw_vals.len());
for (i, raw_val) in raw_vals.into_iter().enumerate() {
let raw_val = RawOsString::new(raw_val);
if !raw_val.contains(val_delim)
|| (self.cmd.is_dont_delimit_trailing_values_set()
&& trailing_idx == Some(i))
{
split_raw_vals.push(raw_val.into_os_string());
split_raw_vals.push(raw_val);
} else {
split_raw_vals
.extend(raw_val.split(val_delim).map(|x| x.to_os_str().into_owned()));
split_raw_vals.extend(raw_val.split(val_delim).map(|x| x.to_owned()));
}
}
raw_vals = split_raw_vals
Expand Down
3 changes: 1 addition & 2 deletions clap_complete/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ bench = false
clap = { path = "../", version = "4.1.0", default-features = false, features = ["std"] }
clap_lex = { path = "../clap_lex", version = "0.3.0", optional = true }
is_executable = { version = "1.0.1", optional = true }
os_str_bytes = { version = "6.0.0", default-features = false, features = ["raw_os_str"], optional = true }
pathdiff = { version = "0.2.1", optional = true }
shlex = { version = "1.1.0", optional = true }
unicode-xid = { version = "0.2.2", optional = true }
Expand All @@ -52,5 +51,5 @@ required-features = ["unstable-dynamic"]

[features]
default = []
unstable-dynamic = ["dep:clap_lex", "dep:shlex", "dep:unicode-xid", "dep:os_str_bytes", "clap/derive", "dep:is_executable", "dep:pathdiff"]
unstable-dynamic = ["dep:clap_lex", "dep:shlex", "dep:unicode-xid", "clap/derive", "dep:is_executable", "dep:pathdiff"]
debug = ["clap/debug"]
58 changes: 26 additions & 32 deletions clap_complete/src/dynamic.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

/// Complete commands within bash
pub mod bash {
use std::ffi::OsStr;
use std::ffi::OsString;
use std::io::Write;

use clap_lex::OsStrExt as _;
use unicode_xid::UnicodeXID;

#[derive(clap::Subcommand)]
Expand Down Expand Up @@ -320,11 +322,7 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES
return complete_arg(&arg, current_cmd, current_dir, pos_index, is_escaped);
}

debug!(
"complete::next: Begin parsing '{:?}' ({:?})",
arg.to_value_os(),
arg.to_value_os().as_raw_bytes()
);
debug!("complete::next: Begin parsing '{:?}'", arg.to_value_os(),);

if let Ok(value) = arg.to_value() {
if let Some(next_cmd) = current_cmd.find_subcommand(value) {
Expand Down Expand Up @@ -370,28 +368,23 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES

if !is_escaped {
if let Some((flag, value)) = arg.to_long() {
if let Ok(flag) = flag {
if let Some(value) = value {
if let Some(arg) = cmd.get_arguments().find(|a| a.get_long() == Some(flag))
{
completions.extend(
complete_arg_value(value.to_str().ok_or(value), arg, current_dir)
.into_iter()
.map(|os| {
// HACK: Need better `OsStr` manipulation
format!("--{}={}", flag, os.to_string_lossy()).into()
}),
)
}
} else {
if let Some(value) = value {
if let Some(arg) = cmd.get_arguments().find(|a| a.get_long() == Some(flag)) {
completions.extend(
crate::generator::utils::longs_and_visible_aliases(cmd)
complete_arg_value(value.to_str().ok_or(value), arg, current_dir)
.into_iter()
.filter_map(|f| {
f.starts_with(flag).then(|| format!("--{}", f).into())
.map(|os| {
// HACK: Need better `OsStr` manipulation
format!("--{}={}", flag, os.to_string_lossy()).into()
}),
);
)
}
} else {
completions.extend(
crate::generator::utils::longs_and_visible_aliases(cmd)
.into_iter()
.filter_map(|f| f.starts_with(flag).then(|| format!("--{}", f).into())),
);
}
} else if arg.is_escape() || arg.is_stdio() || arg.is_empty() {
// HACK: Assuming knowledge of is_escape / is_stdio
Expand All @@ -408,7 +401,7 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES
crate::generator::utils::shorts_and_visible_aliases(cmd)
.into_iter()
// HACK: Need better `OsStr` manipulation
.map(|f| format!("{}{}", arg.to_value_os().to_str_lossy(), f).into()),
.map(|f| format!("{}{}", arg.to_value_os().to_string_lossy(), f).into()),
);
}
}
Expand All @@ -428,7 +421,7 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES
}

fn complete_arg_value(
value: Result<&str, &clap_lex::RawOsStr>,
value: Result<&str, &OsStr>,
arg: &clap::Arg,
current_dir: Option<&std::path::Path>,
) -> Vec<OsString> {
Expand All @@ -444,7 +437,7 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES
}
} else {
let value_os = match value {
Ok(value) => clap_lex::RawOsStr::from_str(value),
Ok(value) => OsStr::new(value),
Err(value_os) => value_os,
};
match arg.get_value_hint() {
Expand Down Expand Up @@ -485,7 +478,7 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES
}

fn complete_path(
value_os: &clap_lex::RawOsStr,
value_os: &OsStr,
current_dir: Option<&std::path::Path>,
is_wanted: impl Fn(&std::path::Path) -> bool,
) -> Vec<OsString> {
Expand All @@ -499,19 +492,20 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES
}
};
let (existing, prefix) = value_os
.split_once('\\')
.unwrap_or((clap_lex::RawOsStr::from_str(""), value_os));
let root = current_dir.join(existing.to_os_str());
.split_once("\\")
.unwrap_or((OsStr::new(""), value_os));
let root = current_dir.join(existing);
debug!("complete_path: root={:?}, prefix={:?}", root, prefix);
let prefix = prefix.to_string_lossy();

for entry in std::fs::read_dir(&root)
.ok()
.into_iter()
.flatten()
.filter_map(Result::ok)
{
let raw_file_name = clap_lex::RawOsString::new(entry.file_name());
if !raw_file_name.starts_with_os(prefix) {
let raw_file_name = OsString::from(entry.file_name());
if !raw_file_name.starts_with(&prefix) {
continue;
}

Expand Down
3 changes: 0 additions & 3 deletions clap_lex/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,3 @@ pre-release-replacements = [

[lib]
bench = false

[dependencies]
os_str_bytes = { version = "6.0.0", default-features = false, features = ["raw_os_str"] }
Loading

0 comments on commit 627a94f

Please sign in to comment.