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.

Sign up for GitHub

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

Jump to bottom

perf!(lex): Build faster by removing os_str_bytes #4794

Merged
merged 1 commit into from Mar 27, 2023
Merged

perf!(lex): Build faster by removing os_str_bytes #4794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions Cargo.lock
View file
Open in desktop

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
Open in desktop
@@ -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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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"] }