-
-
Notifications
You must be signed in to change notification settings - Fork 40
feat: enable wasm32 compilation by making xx crate conditional
#641
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -2,15 +2,15 @@ use std::collections::BTreeMap; | |
| use std::env; | ||
| use std::fmt::Debug; | ||
| use std::path::{Path, PathBuf}; | ||
| #[cfg(not(target_arch = "wasm32"))] | ||
| use std::process::Command; | ||
| use std::sync::Arc; | ||
|
|
||
| use clap::Args; | ||
| use itertools::Itertools; | ||
| use miette::IntoDiagnostic; | ||
| use regex::Regex; | ||
| use std::sync::LazyLock; | ||
| use xx::process::check_status; | ||
| use xx::{regex, XXError, XXResult}; | ||
|
|
||
| use usage::{Spec, SpecArg, SpecCommand, SpecComplete, SpecFlag}; | ||
|
|
||
|
|
@@ -290,7 +290,8 @@ impl CompleteWord { | |
| trace!("run: {run}"); | ||
| let stdout = sh(&run)?; | ||
| // trace!("stdout: {stdout}"); | ||
| let re = regex!(r"[^\\]:"); | ||
| static COLON_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"[^\\]:").unwrap()); | ||
|
Contributor
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. |
||
| let re = &*COLON_RE; | ||
| return Ok(stdout | ||
| .lines() | ||
| .map(|l| { | ||
|
|
@@ -379,10 +380,6 @@ fn zsh_escape(s: &str) -> String { | |
| .replace(']', "\\]") | ||
| } | ||
|
|
||
| /// Wrap a completion value in single quotes if any character would otherwise | ||
| /// be interpreted by the shell. The result is meant to be inserted by | ||
| /// `compadd -Q` verbatim, so the user sees consistent single-quote quoting | ||
| /// instead of zsh's default mix of backslash and single-quote styles. | ||
| fn zsh_shell_quote(s: &str) -> String { | ||
| fn safe(c: char) -> bool { | ||
| matches!(c, | ||
|
Comment on lines
383
to
385
Contributor
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. The documentation comments for /// Wrap a completion value in single quotes if any character would otherwise
/// be interpreted by the shell. The result is meant to be inserted by
/// compadd -Q verbatim, so the user sees consistent single-quote quoting
/// instead of zsh's default mix of backslash and single-quote styles.
fn zsh_shell_quote(s: &str) -> String {References
|
||
|
|
@@ -393,12 +390,14 @@ fn zsh_shell_quote(s: &str) -> String { | |
| if !s.is_empty() && s.chars().all(safe) { | ||
| return s.to_string(); | ||
| } | ||
| // Wrap in single quotes; close-open dance escapes any internal apostrophes. | ||
| let escaped = s.replace('\'', "'\\''"); | ||
| format!("'{escaped}'") | ||
|
Comment on lines
383
to
394
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.
The three-line |
||
| } | ||
|
|
||
| fn sh(script: &str) -> XXResult<String> { | ||
| #[cfg(not(target_arch = "wasm32"))] | ||
| fn sh(script: &str) -> xx::XXResult<String> { | ||
| use xx::process::check_status; | ||
| use xx::XXError; | ||
| let output = Command::new("sh") | ||
| .arg("-c") | ||
| .arg(script) | ||
|
|
@@ -413,3 +412,8 @@ fn sh(script: &str) -> XXResult<String> { | |
| let stdout = String::from_utf8(output.stdout).expect("stdout is not utf-8"); | ||
| Ok(stdout) | ||
| } | ||
|
|
||
| #[cfg(target_arch = "wasm32")] | ||
| fn sh(_script: &str) -> miette::Result<String> { | ||
| Err(miette::miette!("shell execution is not supported on wasm")) | ||
| } | ||
|
Comment on lines
397
to
+419
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.
The non-wasm32 variant returns Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -356,7 +356,11 @@ impl Fig { | |
| pub fn run(&self) -> miette::Result<()> { | ||
| let write = |path: &PathBuf, md: &str| -> miette::Result<()> { | ||
| println!("writing to {}", path.display()); | ||
| xx::file::write(path, format!("{}\n", md.trim()))?; | ||
| if let Some(parent) = path.parent() { | ||
| std::fs::create_dir_all(parent).map_err(|e| miette::miette!("{e}"))?; | ||
| } | ||
| std::fs::write(path, format!("{}\n", md.trim())) | ||
| .map_err(|e| miette::miette!("{e}"))?; | ||
|
Comment on lines
+360
to
+363
Contributor
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. Instead of manually mapping the error to a if let Some(parent) = path.parent() {
std::fs::create_dir_all(parent).into_diagnostic()?;
}
std::fs::write(path, format!("{}\n", md.trim()))
.into_diagnostic()?; |
||
| Ok(()) | ||
| }; | ||
| let spec = generate::file_or_spec(&self.file, &self.spec)?; | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -37,6 +37,7 @@ strum = { version = "0.28", features = ["derive"] } | |||||||||||||||
| tera = { version = "1", optional = true } | ||||||||||||||||
| thiserror = "2" | ||||||||||||||||
| versions = "7" | ||||||||||||||||
| [target.'cfg(not(target_arch = "wasm32"))'.dependencies] | ||||||||||||||||
| xx = "2" | ||||||||||||||||
|
Comment on lines
39
to
41
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.
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
| [features] | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,8 @@ use crate::error::UsageErr; | |
| use itertools::Itertools; | ||
| use serde::Serialize; | ||
| use std::collections::HashMap; | ||
| use xx::regex; | ||
| use regex::Regex; | ||
| use std::sync::LazyLock; | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub struct MarkdownRenderer { | ||
|
|
@@ -88,7 +89,10 @@ impl MarkdownRenderer { | |
| return line.to_string(); | ||
| } | ||
| // replace '<' with '<' but not inside code blocks | ||
| xx::regex!(r"(`[^`]*`)|(<)") | ||
| { | ||
| static RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"(`[^`]*`)|(<)").unwrap()); | ||
| &RE | ||
| } | ||
|
Comment on lines
+93
to
+95
Contributor
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. |
||
| .replace_all(line, |caps: ®ex::Captures| { | ||
| if caps.get(1).is_some() { | ||
| caps.get(1).unwrap().as_str().to_string() | ||
|
|
@@ -102,8 +106,8 @@ impl MarkdownRenderer { | |
| Ok(value.into()) | ||
| }, | ||
| ); | ||
| let path_re = | ||
| regex!(r"https://(github.com/[^/]+/[^/]+|gitlab.com/[^/]+/[^/]+/-)/blob/[^/]+/"); | ||
| static PATH_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"https://(github.com/[^/]+/[^/]+|gitlab.com/[^/]+/[^/]+/-)/blob/[^/]+/").unwrap()); | ||
| let path_re = &*PATH_RE; | ||
| tera.register_function("source_code_link", |args: &HashMap<String, tera::Value>| { | ||
| let spec = args.get("spec").unwrap().as_object().unwrap(); | ||
| let cmd = args.get("cmd").unwrap().as_object().unwrap(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,7 +1,11 @@ | ||||||
| #[cfg(not(target_arch = "wasm32"))] | ||||||
| use std::process::Command; | ||||||
| #[cfg(not(target_arch = "wasm32"))] | ||||||
| use xx::process::check_status; | ||||||
| #[cfg(not(target_arch = "wasm32"))] | ||||||
| use xx::{XXError, XXResult}; | ||||||
|
|
||||||
| #[cfg(not(target_arch = "wasm32"))] | ||||||
| pub(crate) fn sh(script: &str) -> XXResult<String> { | ||||||
|
Contributor
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. The return type of the non-wasm
Suggested change
|
||||||
| #[cfg(unix)] | ||||||
| let (shell, flag) = ("sh", "-c"); | ||||||
|
|
@@ -23,3 +27,11 @@ pub(crate) fn sh(script: &str) -> XXResult<String> { | |||||
| let stdout = String::from_utf8(output.stdout).expect("stdout is not utf-8"); | ||||||
| Ok(stdout) | ||||||
| } | ||||||
|
|
||||||
| #[cfg(target_arch = "wasm32")] | ||||||
| pub(crate) fn sh(_script: &str) -> std::result::Result<String, crate::error::UsageErr> { | ||||||
| Err(crate::error::UsageErr::IO(std::io::Error::new( | ||||||
| std::io::ErrorKind::Unsupported, | ||||||
| "shell execution is not supported on wasm", | ||||||
| ))) | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,8 @@ use std::fmt::{Display, Formatter}; | |
| use std::iter::once; | ||
| use std::path::Path; | ||
| use std::str::FromStr; | ||
| use xx::file; | ||
| use regex::Regex; | ||
| use std::sync::LazyLock; | ||
|
|
||
| use crate::error::UsageErr; | ||
| use crate::spec::cmd::{SpecCommand, SpecExample}; | ||
|
|
@@ -102,7 +103,7 @@ impl Spec { | |
| /// If `bin` is not specified in the spec, it defaults to the filename. | ||
| #[must_use = "parsing result should be used"] | ||
| pub fn parse_script(file: &Path) -> Result<Spec, UsageErr> { | ||
| let raw = extract_usage_from_comments(&file::read_to_string(file)?); | ||
| let raw = extract_usage_from_comments(&std::fs::read_to_string(file)?); | ||
| let ctx = ParsingContext::new(file, &raw); | ||
| let mut spec = Self::parse(&ctx, &raw)?; | ||
| if spec.bin.is_empty() { | ||
|
|
@@ -303,11 +304,11 @@ fn check_usage_version(version: &str) { | |
| } | ||
|
|
||
| fn split_script(file: &Path) -> Result<String, UsageErr> { | ||
| let full = file::read_to_string(file)?; | ||
| let full = std::fs::read_to_string(file)?; | ||
| // If file has a shebang and USAGE comments, extract the spec from comments | ||
| if full.starts_with("#!") { | ||
| let usage_regex = xx::regex!(r"^(?:#|//|::)(?:USAGE| ?\[USAGE\])"); | ||
| if full.lines().any(|l| usage_regex.is_match(l)) { | ||
| static USAGE_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"^(?:#|//|::)(?:USAGE| ?\[USAGE\])").unwrap()); | ||
|
Contributor
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. |
||
| if full.lines().any(|l| USAGE_RE.is_match(l)) { | ||
| return Ok(extract_usage_from_comments(&full)); | ||
| } | ||
| } | ||
|
|
@@ -316,8 +317,10 @@ fn split_script(file: &Path) -> Result<String, UsageErr> { | |
| } | ||
|
|
||
| fn extract_usage_from_comments(full: &str) -> String { | ||
| let usage_regex = xx::regex!(r"^(?:#|//|::)(?:USAGE| ?\[USAGE\])(.*)$"); | ||
| let blank_comment_regex = xx::regex!(r"^(?:#|//|::)\s*$"); | ||
| static USAGE_CAPTURE_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"^(?:#|//|::)(?:USAGE| ?\[USAGE\])(.*)$").unwrap()); | ||
| static BLANK_COMMENT_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"^(?:#|//|::)\s*$").unwrap()); | ||
| let usage_regex = &*USAGE_CAPTURE_RE; | ||
| let blank_comment_regex = &*BLANK_COMMENT_RE; | ||
| let mut usage = vec![]; | ||
| let mut found = false; | ||
| for line in full.lines() { | ||
|
|
||
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.
[target.…]section makes the TOML harder to read and deviates from the project's existing style (compare with the[target.'cfg(unix)'.dependencies]section below, which has a blank line before it).Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!