From cc91e36cf8aaf1411d55e6e8510e644fdb1e3d29 Mon Sep 17 00:00:00 2001 From: Justin Ma Date: Thu, 2 May 2024 11:11:53 +0800 Subject: [PATCH 01/16] Upgrade Nu to v0.93.0 for nightly and release workflow (#12721) --- .github/workflows/nightly-build.yml | 8 ++++---- .github/workflows/release.yml | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/nightly-build.yml b/.github/workflows/nightly-build.yml index ca4cb04a2c..f86d7572a2 100644 --- a/.github/workflows/nightly-build.yml +++ b/.github/workflows/nightly-build.yml @@ -39,7 +39,7 @@ jobs: uses: hustcer/setup-nu@v3.10 if: github.repository == 'nushell/nightly' with: - version: 0.91.0 + version: 0.93.0 # Synchronize the main branch of nightly repo with the main branch of Nushell official repo - name: Prepare for Nightly Release @@ -141,7 +141,7 @@ jobs: - name: Setup Nushell uses: hustcer/setup-nu@v3.10 with: - version: 0.91.0 + version: 0.93.0 - name: Release Nu Binary id: nu @@ -253,7 +253,7 @@ jobs: - name: Setup Nushell uses: hustcer/setup-nu@v3.10 with: - version: 0.91.0 + version: 0.93.0 - name: Release Nu Binary id: nu @@ -317,7 +317,7 @@ jobs: - name: Setup Nushell uses: hustcer/setup-nu@v3.10 with: - version: 0.91.0 + version: 0.93.0 # Keep the last a few releases - name: Delete Older Releases diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e08b96cb0f..fce38d9b4b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -89,7 +89,7 @@ jobs: - name: Setup Nushell uses: hustcer/setup-nu@v3.10 with: - version: 0.91.0 + version: 0.93.0 - name: Release Nu Binary id: nu @@ -179,7 +179,7 @@ jobs: - name: Setup Nushell uses: hustcer/setup-nu@v3.10 with: - version: 0.91.0 + version: 0.93.0 - name: Release Nu Binary id: nu From b5741ef14bb094fd8e1739c1783059eba69ebe0a Mon Sep 17 00:00:00 2001 From: Reilly Wood <26268125+rgwood@users.noreply.github.com> Date: Thu, 2 May 2024 06:14:36 -0700 Subject: [PATCH 02/16] Remove accidentally committed file (#12734) Remove a small (20kb) SQLite database that was accidentally added as part of https://github.com/nushell/nushell/pull/12692 --- foo.db | Bin 20480 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 foo.db diff --git a/foo.db b/foo.db deleted file mode 100644 index 7fadc66cfa9740c335f51d6b8a4ef015251e4ed8..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 20480 zcmeHOd2kzN6@R-cS%-DyXw$^8wXvPpPQ1D=yG^UaQPQ-E>?BSbr!=-Kt!>#lw%V1C z6lelN2{2HXa)jGZ2Bs;8+d@m}OfZFkGA%Ps;SUCw;=+_E(+&y4(J6(2x2x>xsO*@4 zXZT|^V@vY;z3;vE`@VhOe*2z}9*(92mLAFFGjW0Tf(-})!ZZy48vN1XkNhI>gZMZ3 zMaUmkc-Meq@~jyFq_q{SU&deROJzW1KxIH>KxIH>KxIH>KxIH>KxIH>KxN?nl!14X zx=riW5obn)_;8x#c#fxfjzsqKN9g`Nd!rHhE{?yOZs$@-`e3X-av*Y~ljbVEW#bu^ z?vEVr@1%JlE)@7HUlLq_#(|=pbcW58lqXYpp%72Yf9N z!VIU_OPylnfzV7YnHov4NqQf8-+%B>q(eNLZ>;|eST9)bv+lDREsK^PuxnWk6*>Wk6*>Wk6*>Wk6*>Wne8BaBb9X*gzhg zN=+wIc^i0K9v|cKF`gjp4t2YO-JYPs?+fBzU9J%03U=e$2JN=%$fJ1spW_`uE|VTf zrCH+mYBbjuHFLT2L`raE3!>)DpQx$n4Y<2pf#4OJo^{$y=%v8&0>8YUl{#LVcC)Ax z&yTV~nUWX7pxuFPzq{MzbA&b0)7{0W}|eADs~OU4qlY%srO{-ODtdBS|7S!?>O=}FV9Y1|Ys zU0?sF`p?wQ)c4nKuLt9Aj0?su8sB4d8*PRK!xs$q8tyRo3>)=-*1umrs^72srS7}B zPw5z)x$gD4({<~$U(q&c{-Swa^CiuUrj2@-%28pemC}>1lP{2uk{>2>NVP_H>SSr9Lr@`XP!-^(=$%TB%jD7S%zoxlWg8OoXR>`@#^q< zMuGtkCc`9~2qfa3L?X}?AC9~IL01qz`IFu*XBHKYrdgItWk)yn(nxfEeh9{Im#t~0 ze!fQ5G#v}z3(mv%Z6#aZo_cIeBGa)n-1r6uTMz0C`Yg%_f|j zw38-f4{r~HrOySPFv54^64Tt>Q3;PHCgP(kZoY~NVLc7&jk9ouxJ8`4W1?2?6S1+% z=@a*<0_RkAD&FaK1cMF_GaU*H27F8*JCV&zWtns;TbO1>vjxRNX@`qvADQmxg}`ZjU#TCM!bLOyu^igRh=r$dZ-WJ`?Jt?lLCBSZD33UE<)Rl3NTw6*oPP2 z;|tKwcS_8r+&*0*>{qRkHZxJos*~K{_bg(#9fd zWNY2UB}{|kjbRuM$ksHmn`66IBKVq3#0c0bO$lpkWdV>eY?$%G9s2{ifS z1QNTcDuG~vS%7hG$yra#y*AFCqhb4Y0J}$V)*W-za@MUl>#FIjOK~=OPn25|XUU}n zXnp4_*qySoQ7)RR<*Y+?HkzyHEK_p!uIP!iarR|&cIGtLI~8Y7Lh^>J%1(A=!8zP6*(UYE zEC4+C^boXcQ35_j^j8PGX&#Jy81QB#;Qo~Y&SSf<5$~;TPy+6%5wIUXScKM1IHtJa zQ}BwD7n3>O?+C(-?BY5OhE7B4^$O~DHBl}&F$X)ZLlh&fdRVHCS{{$+7=pTQorY~2 z5o?$1z9daXIn~Wez`MY=0U&W2!9*`;$IwW zw<1=$m4b8Os!c9Kvm)UIyG*4WIz_hrw=?0Y|CHCj%nY0Fo0;c(e%EBhF*6COrl5 z5sp0h$LD|;2tz=FIufTXHFd^wg*@JgD{64j2Gnbq-Y7j$Jkr#W9NtlO$>Wlkj8DT> z5{H%UXe#%q4X2@H4w?zQL63PzqbXiV6(^<#G5>)kD%Ss_c+9r`8an{Kpq~HL^ZzwI z*-+2_>iJ*lZBozw>iPd_k3rS*|FTbrDfRrXp8rP)_581%|JCzIDi>&L7+EdQ|l%yQmx#^N%6)qKAhOb?j^Q@_b)+F1XU z`VZ7M7{6}3#kkS%2g3!!Lx!-SN&lw)Mg2MbN&R-+>$-30PU!}80p0q#OLZ^QJyLf^ z9aCr1{#pB?b{>aUUn&DC11bY516N_7>0T_5NbNJ!C62n3D^-Qeb7B(=eG27LK@f#+ zFVMAI2K$VjDn2qsaphxSY$m}fcEhxuDz?E;pZ#n_MG@rwG9yPyVwC0uL+# zJde(Bh>@PVBY-&h_X?s2o7ik*&&kt(^2%u_b}NOlI0lvx(9TYlXzx}^NO4YZW>_p4 zCwV6?W{5xg33ED=2^wDpdV>g^ zhmK(d+I(RdX!AVwN6kS;yafFs(fpxW4%|5husDnE8YoJtnP?D5H2W*#uk4T#Pjmi+ zmknUoWWdV?yFzh)JmDGfx<^Kmu4E|S4Fs?|DwIt45*1yO13PXs{pCQ)1IBwi0~e_tU$x1vy)hSXS@5-v(#u`bAQtMnv>+ zWl~n%iHQGDQfv#IzDg(JyZ?rn0iOEN0`wlm_7N%HO=qU}R&w Date: Thu, 2 May 2024 09:36:37 -0400 Subject: [PATCH 03/16] add raw-string literal support (#9956) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description This PR adds raw string support by using `r#` at the beginning of single quoted strings and `#` at the end. Notice that escapes do not process, even within single quotes, parentheses don't mean anything, $variables don't mean anything. It's just a string. ```nushell ❯ echo r#'one\ntwo (blah) ($var)'# one\ntwo (blah) ($var) ``` Notice how they work without `echo` or `print` and how they work without carriage returns. ```nushell ❯ r#'adsfa'# adsfa ❯ r##"asdfa'@qpejq'## asdfa'@qpejq ❯ r#'asdfasdfasf ∙ foqwejfqo@'23rfjqf'# ``` They also have a special configurable color in the repl. (use single quotes though) ![image](https://github.com/nushell/nushell/assets/343840/8780e21d-de4c-45b3-9880-2425f5fe10ef) They should work like rust raw literals and allow `r##`, `r###`, `r####`, etc, to help with having one or many `#`'s in the middle of your raw-string. They should work with `let` as well. ```nushell r#'some\nraw\nstring'# | str upcase ``` closes https://github.com/nushell/nushell/issues/5091 # User-Facing Changes # Tests + Formatting # After Submitting --------- Co-authored-by: WindSoilder Co-authored-by: Ian Manske --- crates/nu-cli/src/syntax_highlight.rs | 2 + crates/nu-color-config/src/shape_color.rs | 1 + crates/nu-command/tests/commands/let_.rs | 15 ++++ crates/nu-command/tests/commands/mut_.rs | 15 ++++ crates/nu-parser/src/flatten.rs | 5 ++ crates/nu-parser/src/lex.rs | 73 ++++++++++++++++++ crates/nu-parser/src/parse_keywords.rs | 1 + crates/nu-parser/src/parser.rs | 75 +++++++++++++++++++ crates/nu-protocol/src/ast/expr.rs | 2 + crates/nu-protocol/src/ast/expression.rs | 2 + crates/nu-protocol/src/debugger/profiler.rs | 2 +- crates/nu-protocol/src/eval_base.rs | 2 +- .../src/sample_config/default_config.nu | 2 + crates/nuon/src/from.rs | 2 +- src/ide.rs | 10 +++ src/tests/test_strings.rs | 20 +++++ tests/const_/mod.rs | 15 ++++ 17 files changed, 241 insertions(+), 3 deletions(-) diff --git a/crates/nu-cli/src/syntax_highlight.rs b/crates/nu-cli/src/syntax_highlight.rs index 8d0c582bd1..da293f03ac 100644 --- a/crates/nu-cli/src/syntax_highlight.rs +++ b/crates/nu-cli/src/syntax_highlight.rs @@ -128,6 +128,7 @@ impl Highlighter for NuHighlighter { FlatShape::Operator => add_colored_token(&shape.1, next_token), FlatShape::Signature => add_colored_token(&shape.1, next_token), FlatShape::String => add_colored_token(&shape.1, next_token), + FlatShape::RawString => add_colored_token(&shape.1, next_token), FlatShape::StringInterpolation => add_colored_token(&shape.1, next_token), FlatShape::DateTime => add_colored_token(&shape.1, next_token), FlatShape::List => { @@ -353,6 +354,7 @@ fn find_matching_block_end_in_expr( Expr::Directory(_, _) => None, Expr::GlobPattern(_, _) => None, Expr::String(_) => None, + Expr::RawString(_) => None, Expr::CellPath(_) => None, Expr::ImportPattern(_) => None, Expr::Overlay(_) => None, diff --git a/crates/nu-color-config/src/shape_color.rs b/crates/nu-color-config/src/shape_color.rs index 188abda992..cb896f2bfc 100644 --- a/crates/nu-color-config/src/shape_color.rs +++ b/crates/nu-color-config/src/shape_color.rs @@ -32,6 +32,7 @@ pub fn default_shape_color(shape: String) -> Style { "shape_or" => Style::new().fg(Color::Purple).bold(), "shape_pipe" => Style::new().fg(Color::Purple).bold(), "shape_range" => Style::new().fg(Color::Yellow).bold(), + "shape_raw_string" => Style::new().fg(Color::LightMagenta).bold(), "shape_record" => Style::new().fg(Color::Cyan).bold(), "shape_redirection" => Style::new().fg(Color::Purple).bold(), "shape_signature" => Style::new().fg(Color::Green).bold(), diff --git a/crates/nu-command/tests/commands/let_.rs b/crates/nu-command/tests/commands/let_.rs index a9a6c4b3b1..4bedf31104 100644 --- a/crates/nu-command/tests/commands/let_.rs +++ b/crates/nu-command/tests/commands/let_.rs @@ -91,3 +91,18 @@ fn let_glob_type() { let actual = nu!("let x: glob = 'aa'; $x | describe"); assert_eq!(actual.out, "glob"); } + +#[test] +fn let_raw_string() { + let actual = nu!(r#"let x = r#'abcde""fghi"''''jkl'#; $x"#); + assert_eq!(actual.out, r#"abcde""fghi"''''jkl"#); + + let actual = nu!(r#"let x = r##'abcde""fghi"''''#jkl'##; $x"#); + assert_eq!(actual.out, r#"abcde""fghi"''''#jkl"#); + + let actual = nu!(r#"let x = r###'abcde""fghi"'''##'#jkl'###; $x"#); + assert_eq!(actual.out, r#"abcde""fghi"'''##'#jkl"#); + + let actual = nu!(r#"let x = r#'abc'#; $x"#); + assert_eq!(actual.out, "abc"); +} diff --git a/crates/nu-command/tests/commands/mut_.rs b/crates/nu-command/tests/commands/mut_.rs index be2d588ab0..7078cd1df1 100644 --- a/crates/nu-command/tests/commands/mut_.rs +++ b/crates/nu-command/tests/commands/mut_.rs @@ -125,3 +125,18 @@ fn mut_glob_type() { let actual = nu!("mut x: glob = 'aa'; $x | describe"); assert_eq!(actual.out, "glob"); } + +#[test] +fn mut_raw_string() { + let actual = nu!(r#"mut x = r#'abcde""fghi"''''jkl'#; $x"#); + assert_eq!(actual.out, r#"abcde""fghi"''''jkl"#); + + let actual = nu!(r#"mut x = r##'abcde""fghi"''''#jkl'##; $x"#); + assert_eq!(actual.out, r#"abcde""fghi"''''#jkl"#); + + let actual = nu!(r#"mut x = r###'abcde""fghi"'''##'#jkl'###; $x"#); + assert_eq!(actual.out, r#"abcde""fghi"'''##'#jkl"#); + + let actual = nu!(r#"mut x = r#'abc'#; $x"#); + assert_eq!(actual.out, "abc"); +} diff --git a/crates/nu-parser/src/flatten.rs b/crates/nu-parser/src/flatten.rs index 0f99efb6fb..cb1d1de110 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -38,6 +38,7 @@ pub enum FlatShape { Or, Pipe, Range, + RawString, Record, Redirection, Signature, @@ -78,6 +79,7 @@ impl Display for FlatShape { FlatShape::Or => write!(f, "shape_or"), FlatShape::Pipe => write!(f, "shape_pipe"), FlatShape::Range => write!(f, "shape_range"), + FlatShape::RawString => write!(f, "shape_raw_string"), FlatShape::Record => write!(f, "shape_record"), FlatShape::Redirection => write!(f, "shape_redirection"), FlatShape::Signature => write!(f, "shape_signature"), @@ -509,6 +511,9 @@ pub fn flatten_expression( Expr::String(_) => { vec![(expr.span, FlatShape::String)] } + Expr::RawString(_) => { + vec![(expr.span, FlatShape::RawString)] + } Expr::Table(table) => { let outer_span = expr.span; let mut last_end = outer_span.start; diff --git a/crates/nu-parser/src/lex.rs b/crates/nu-parser/src/lex.rs index 399afb428e..11afea861d 100644 --- a/crates/nu-parser/src/lex.rs +++ b/crates/nu-parser/src/lex.rs @@ -503,6 +503,79 @@ fn lex_internal( } else if c == b' ' || c == b'\t' || additional_whitespace.contains(&c) { // If the next character is non-newline whitespace, skip it. curr_offset += 1; + } else if c == b'r' { + // A raw string literal looks like `echo r#'Look, I can use 'single quotes'!'#` + // If the next character is `#` we're probably looking at a raw string literal + // so we need to read all the text until we find a closing `#`. This raw string + // can contain any character, including newlines and double quotes without needing + // to escape them. + // + // A raw string can contain many `#` as prefix, + // incase if there is a `'#` or `#'` in the string itself. + // E.g: r##'I can use '#' in a raw string'## + let mut prefix_sharp_cnt = 0; + let start = curr_offset; + while let Some(b'#') = input.get(start + prefix_sharp_cnt + 1) { + prefix_sharp_cnt += 1; + } + + if prefix_sharp_cnt != 0 { + // curr_offset is the character `r`, we need to move forward and skip all `#` + // characters. + // + // e.g: r###' + // ^ + // ^ + // curr_offset + curr_offset += prefix_sharp_cnt + 1; + // the next one should be a single quote. + if input.get(curr_offset) != Some(&b'\'') { + error = Some(ParseError::Expected( + "'", + Span::new(span_offset + curr_offset, span_offset + curr_offset + 1), + )); + } + + curr_offset += 1; + let mut matches = false; + while let Some(ch) = input.get(curr_offset) { + // check for postfix '### + if *ch == b'#' { + let start_ch = input[curr_offset - prefix_sharp_cnt]; + let postfix = &input[curr_offset - prefix_sharp_cnt + 1..=curr_offset]; + if start_ch == b'\'' && postfix.iter().all(|x| *x == b'#') { + matches = true; + curr_offset += 1; + break; + } + } + curr_offset += 1 + } + if matches { + output.push(Token::new( + TokenContents::Item, + Span::new(span_offset + start, span_offset + curr_offset), + )); + } else if error.is_none() { + error = Some(ParseError::UnexpectedEof( + "#".to_string(), + Span::new(span_offset + curr_offset, span_offset + curr_offset), + )) + } + } else { + let (token, err) = lex_item( + input, + &mut curr_offset, + span_offset, + additional_whitespace, + special_tokens, + in_signature, + ); + if error.is_none() { + error = err; + } + output.push(token); + } } else { let token = try_lex_special_piped_item(input, &mut curr_offset, span_offset); if let Some(token) = token { diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 9b0b552259..4c2b2f7bd8 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -3341,6 +3341,7 @@ pub fn parse_mut(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline } pub fn parse_source(working_set: &mut StateWorkingSet, lite_command: &LiteCommand) -> Pipeline { + trace!("parsing source"); let spans = &lite_command.parts; let name = working_set.get_span_contents(spans[0]); diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index cca0eaaa2c..19b84c1e75 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -66,6 +66,11 @@ pub fn is_math_expression_like(working_set: &mut StateWorkingSet, span: Span) -> let b = bytes[0]; + // check for raw string + if bytes.starts_with(b"r#") { + return true; + } + if b == b'(' || b == b'{' || b == b'[' || b == b'$' || b == b'"' || b == b'\'' || b == b'-' { return true; } @@ -578,6 +583,7 @@ pub fn parse_multispan_value( spans_idx: &mut usize, shape: &SyntaxShape, ) -> Expression { + trace!("parse multispan value"); match shape { SyntaxShape::VarWithOptType => { trace!("parsing: var with opt type"); @@ -1565,6 +1571,66 @@ pub(crate) fn parse_dollar_expr(working_set: &mut StateWorkingSet, span: Span) - } } +pub fn parse_raw_string(working_set: &mut StateWorkingSet, span: Span) -> Expression { + trace!("parsing: raw-string, with required delimiters"); + + let bytes = working_set.get_span_contents(span); + + let prefix_sharp_cnt = if bytes.starts_with(b"r#") { + // actually `sharp_cnt` is always `index - 1` + // but create a variable here to make it clearer. + let mut sharp_cnt = 1; + let mut index = 2; + while index < bytes.len() && bytes[index] == b'#' { + index += 1; + sharp_cnt += 1; + } + sharp_cnt + } else { + working_set.error(ParseError::Expected("r#", span)); + return garbage(span); + }; + let expect_postfix_sharp_cnt = prefix_sharp_cnt; + // check the length of whole raw string. + // the whole raw string should contains at least + // 1(r) + prefix_sharp_cnt + 1(') + 1(') + postfix_sharp characters + if bytes.len() < prefix_sharp_cnt + expect_postfix_sharp_cnt + 3 { + working_set.error(ParseError::Unclosed('\''.into(), span)); + return garbage(span); + } + + // check for unbalanced # and single quotes. + let postfix_bytes = &bytes[bytes.len() - expect_postfix_sharp_cnt..bytes.len()]; + if postfix_bytes.iter().any(|b| *b != b'#') { + working_set.error(ParseError::Unbalanced( + "prefix #".to_string(), + "postfix #".to_string(), + span, + )); + return garbage(span); + } + // check for unblanaced single quotes. + if bytes[1 + prefix_sharp_cnt] != b'\'' + || bytes[bytes.len() - expect_postfix_sharp_cnt - 1] != b'\'' + { + working_set.error(ParseError::Unclosed('\''.into(), span)); + return garbage(span); + } + + let bytes = &bytes[prefix_sharp_cnt + 1 + 1..bytes.len() - 1 - prefix_sharp_cnt]; + if let Ok(token) = String::from_utf8(bytes.into()) { + Expression { + expr: Expr::RawString(token), + span, + ty: Type::String, + custom_completion: None, + } + } else { + working_set.error(ParseError::Expected("utf8 raw-string", span)); + garbage(span) + } +} + pub fn parse_paren_expr( working_set: &mut StateWorkingSet, span: Span, @@ -4553,6 +4619,9 @@ pub fn parse_value( return Expression::garbage(span); } }, + b'r' if bytes.len() > 1 && bytes[1] == b'#' => { + return parse_raw_string(working_set, span); + } _ => {} } @@ -6075,6 +6144,7 @@ pub fn discover_captures_in_expr( } } Expr::String(_) => {} + Expr::RawString(_) => {} Expr::StringInterpolation(exprs) => { for expr in exprs { discover_captures_in_expr(working_set, expr, seen, seen_blocks, output)?; @@ -6236,6 +6306,7 @@ pub fn parse( contents: &[u8], scoped: bool, ) -> Arc { + trace!("parse"); let name = match fname { Some(fname) => { // use the canonical name for this filename @@ -6253,9 +6324,13 @@ pub fn parse( let mut output = { if let Some(block) = previously_parsed_block { + // dbg!("previous block"); return block; } else { + // dbg!("starting lex"); let (output, err) = lex(contents, new_span.start, &[], &[], false); + // dbg!("finished lex"); + // dbg!(&output); if let Some(err) = err { working_set.error(err) } diff --git a/crates/nu-protocol/src/ast/expr.rs b/crates/nu-protocol/src/ast/expr.rs index 53a0717f34..13d9e42985 100644 --- a/crates/nu-protocol/src/ast/expr.rs +++ b/crates/nu-protocol/src/ast/expr.rs @@ -36,6 +36,7 @@ pub enum Expr { Directory(String, bool), GlobPattern(String, bool), String(String), + RawString(String), CellPath(CellPath), FullCellPath(Box), ImportPattern(Box), @@ -80,6 +81,7 @@ impl Expr { | Expr::ValueWithUnit(_) | Expr::DateTime(_) | Expr::String(_) + | Expr::RawString(_) | Expr::CellPath(_) | Expr::StringInterpolation(_) | Expr::Nothing => { diff --git a/crates/nu-protocol/src/ast/expression.rs b/crates/nu-protocol/src/ast/expression.rs index 2f31196871..8fdbc8567c 100644 --- a/crates/nu-protocol/src/ast/expression.rs +++ b/crates/nu-protocol/src/ast/expression.rs @@ -279,6 +279,7 @@ impl Expression { } Expr::Signature(_) => false, Expr::String(_) => false, + Expr::RawString(_) => false, Expr::RowCondition(block_id) | Expr::Subexpression(block_id) => { let block = working_set.get_block(*block_id); @@ -436,6 +437,7 @@ impl Expression { } Expr::Signature(_) => {} Expr::String(_) => {} + Expr::RawString(_) => {} Expr::StringInterpolation(items) => { for i in items { i.replace_span(working_set, replaced, new_span) diff --git a/crates/nu-protocol/src/debugger/profiler.rs b/crates/nu-protocol/src/debugger/profiler.rs index d1efe90cb0..9d5bece0ab 100644 --- a/crates/nu-protocol/src/debugger/profiler.rs +++ b/crates/nu-protocol/src/debugger/profiler.rs @@ -253,7 +253,7 @@ fn expr_to_string(engine_state: &EngineState, expr: &Expr) -> String { Expr::Record(_) => "record".to_string(), Expr::RowCondition(_) => "row condition".to_string(), Expr::Signature(_) => "signature".to_string(), - Expr::String(_) => "string".to_string(), + Expr::String(_) | Expr::RawString(_) => "string".to_string(), Expr::StringInterpolation(_) => "string interpolation".to_string(), Expr::Subexpression(_) => "subexpression".to_string(), Expr::Table(_) => "table".to_string(), diff --git a/crates/nu-protocol/src/eval_base.rs b/crates/nu-protocol/src/eval_base.rs index 06d13a5f99..f211f48560 100644 --- a/crates/nu-protocol/src/eval_base.rs +++ b/crates/nu-protocol/src/eval_base.rs @@ -139,7 +139,7 @@ pub trait Eval { Ok(Value::list(output_rows, expr.span)) } Expr::Keyword(kw) => Self::eval::(state, mut_state, &kw.expr), - Expr::String(s) => Ok(Value::string(s.clone(), expr.span)), + Expr::String(s) | Expr::RawString(s) => Ok(Value::string(s.clone(), expr.span)), Expr::Nothing => Ok(Value::nothing(expr.span)), Expr::ValueWithUnit(value) => match Self::eval::(state, mut_state, &value.expr)? { Value::Int { val, .. } => value.unit.item.build_value(val, value.unit.span), diff --git a/crates/nu-utils/src/sample_config/default_config.nu b/crates/nu-utils/src/sample_config/default_config.nu index ca82cb2e8c..70e8a2ca7d 100644 --- a/crates/nu-utils/src/sample_config/default_config.nu +++ b/crates/nu-utils/src/sample_config/default_config.nu @@ -69,6 +69,7 @@ let dark_theme = { shape_table: blue_bold shape_variable: purple shape_vardecl: purple + shape_raw_string: light_purple } let light_theme = { @@ -134,6 +135,7 @@ let light_theme = { shape_table: blue_bold shape_variable: purple shape_vardecl: purple + shape_raw_string: light_purple } # External completer example diff --git a/crates/nuon/src/from.rs b/crates/nuon/src/from.rs index a06a75e3f5..e1ba37b966 100644 --- a/crates/nuon/src/from.rs +++ b/crates/nuon/src/from.rs @@ -319,7 +319,7 @@ fn convert_to_value( msg: "signatures not supported in nuon".into(), span: expr.span, }), - Expr::String(s) => Ok(Value::string(s, span)), + Expr::String(s) | Expr::RawString(s) => Ok(Value::string(s, span)), Expr::StringInterpolation(..) => Err(ShellError::OutsideSpannedLabeledError { src: original_text.to_string(), error: "Error when loading".into(), diff --git a/src/ide.rs b/src/ide.rs index 8e0a60421b..2b39dda946 100644 --- a/src/ide.rs +++ b/src/ide.rs @@ -569,6 +569,16 @@ pub fn hover(engine_state: &mut EngineState, file_path: &str, location: &Value) } }) ), + FlatShape::RawString => println!( + "{}", + json!({ + "hover": "raw-string", + "span": { + "start": span.start - offset, + "end": span.end - offset + } + }) + ), FlatShape::StringInterpolation => println!( "{}", json!({ diff --git a/src/tests/test_strings.rs b/src/tests/test_strings.rs index 762ac7579f..dcb03801b9 100644 --- a/src/tests/test_strings.rs +++ b/src/tests/test_strings.rs @@ -71,3 +71,23 @@ fn case_insensitive_sort_columns() -> TestResult { r#"[{"version":"four","package":"abc"},{"version":"three","package":"abc"},{"version":"two","package":"Abc"}]"#, ) } + +#[test] +fn raw_string() -> TestResult { + run_test(r#"r#'abcde""fghi"''''jkl'#"#, r#"abcde""fghi"''''jkl"#)?; + run_test(r#"r##'abcde""fghi"''''#jkl'##"#, r#"abcde""fghi"''''#jkl"#)?; + run_test( + r#"r###'abcde""fghi"'''##'#jkl'###"#, + r#"abcde""fghi"'''##'#jkl"#, + )?; + run_test("r#''#", "")?; + run_test( + r#"r#'a string with sharp inside # and ends with #'#"#, + "a string with sharp inside # and ends with #", + ) +} + +#[test] +fn incomplete_raw_string() -> TestResult { + fail_test("r#abc", "expected '") +} diff --git a/tests/const_/mod.rs b/tests/const_/mod.rs index 603eaba66c..04db29d3dd 100644 --- a/tests/const_/mod.rs +++ b/tests/const_/mod.rs @@ -400,3 +400,18 @@ fn const_glob_type() { let actual = nu!("const x: glob = 'aa'; $x | describe"); assert_eq!(actual.out, "glob"); } + +#[test] +fn const_raw_string() { + let actual = nu!(r#"const x = r#'abcde""fghi"''''jkl'#; $x"#); + assert_eq!(actual.out, r#"abcde""fghi"''''jkl"#); + + let actual = nu!(r#"const x = r##'abcde""fghi"''''#jkl'##; $x"#); + assert_eq!(actual.out, r#"abcde""fghi"''''#jkl"#); + + let actual = nu!(r#"const x = r###'abcde""fghi"'''##'#jkl'###; $x"#); + assert_eq!(actual.out, r#"abcde""fghi"'''##'#jkl"#); + + let actual = nu!(r#"const x = r#'abc'#; $x"#); + assert_eq!(actual.out, "abc"); +} From 0805f1fd90974f7ad2f3a54228368f88dcba261f Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Thu, 2 May 2024 09:56:50 -0400 Subject: [PATCH 04/16] overhaul shell_integration to enable individual control over ansi escape sequences (#12629) # Description This PR overhauls the shell_integration system by allowing individual control over which ansi escape sequences are used. As we continue to broaden our support for more ansi escape sequences, we can't really have an all-or-nothing strategy. Some ansi escapes cause problems in certain operating systems or terminals. We should allow the user to choose which escapes they want. TODO: * Gather feedback * Should osc7, osc9_9 and osc633p be mutually exclusive? * Is the naming convention for these settings too nerdy osc2, osc7, etc? closes #11301 # User-Facing Changes shell_integration is no longer a boolean value. This is what is supported in the default_config.nu ```nushell shell_integration: { # osc2 abbreviates the path if in the home_dir, sets the tab/window title, shows the running command in the tab/window title osc2: true # osc7 is a way to communicate the path to the terminal, this is helpful for spawning new tabs in the same directory osc7: true # osc8 is also implemented as the deprecated setting ls.show_clickable_links, it shows clickable links in ls output if your terminal supports it osc8: true # osc9_9 is from ConEmu and is starting to get wider support. It's similar to osc7 in that it communicates the path to the terminal osc9_9: false # osc133 is several escapes invented by Final Term which include the supported ones below. # 133;A - Mark prompt start # 133;B - Mark prompt end # 133;C - Mark pre-execution # 133;D;exit - Mark execution finished with exit code # This is used to enable terminals to know where the prompt is, the command is, where the command finishes, and where the output of the command is osc133: true # osc633 is closely related to osc133 but only exists in visual studio code (vscode) and supports their shell integration features # 633;A - Mark prompt start # 633;B - Mark prompt end # 633;C - Mark pre-execution # 633;D;exit - Mark execution finished with exit code # 633;E - NOT IMPLEMENTED - Explicitly set the command line with an optional nonce # 633;P;Cwd= - Mark the current working directory and communicate it to the terminal # and also helps with the run recent menu in vscode osc633: true # reset_application_mode is escape \x1b[?1l and was added to help ssh work better reset_application_mode: true } ``` # Tests + Formatting # After Submitting --- crates/nu-cli/src/prompt.rs | 37 +- crates/nu-cli/src/prompt_update.rs | 65 ++- crates/nu-cli/src/repl.rs | 488 ++++++++++++------ crates/nu-command/src/viewers/table.rs | 6 +- crates/nu-protocol/src/config/mod.rs | 68 ++- .../src/sample_config/default_config.nu | 29 +- 6 files changed, 521 insertions(+), 172 deletions(-) diff --git a/crates/nu-cli/src/prompt.rs b/crates/nu-cli/src/prompt.rs index 0ecdae1aaf..a2045a201c 100644 --- a/crates/nu-cli/src/prompt.rs +++ b/crates/nu-cli/src/prompt.rs @@ -1,4 +1,10 @@ -use crate::prompt_update::{POST_PROMPT_MARKER, PRE_PROMPT_MARKER}; +use crate::prompt_update::{ + POST_PROMPT_MARKER, PRE_PROMPT_MARKER, VSCODE_POST_PROMPT_MARKER, VSCODE_PRE_PROMPT_MARKER, +}; +use nu_protocol::{ + engine::{EngineState, Stack}, + Value, +}; #[cfg(windows)] use nu_utils::enable_vt_processing; use reedline::{ @@ -10,7 +16,8 @@ use std::borrow::Cow; /// Nushell prompt definition #[derive(Clone)] pub struct NushellPrompt { - shell_integration: bool, + shell_integration_osc133: bool, + shell_integration_osc633: bool, left_prompt_string: Option, right_prompt_string: Option, default_prompt_indicator: Option, @@ -18,12 +25,20 @@ pub struct NushellPrompt { default_vi_normal_prompt_indicator: Option, default_multiline_indicator: Option, render_right_prompt_on_last_line: bool, + engine_state: EngineState, + stack: Stack, } impl NushellPrompt { - pub fn new(shell_integration: bool) -> NushellPrompt { + pub fn new( + shell_integration_osc133: bool, + shell_integration_osc633: bool, + engine_state: EngineState, + stack: Stack, + ) -> NushellPrompt { NushellPrompt { - shell_integration, + shell_integration_osc133, + shell_integration_osc633, left_prompt_string: None, right_prompt_string: None, default_prompt_indicator: None, @@ -31,6 +46,8 @@ impl NushellPrompt { default_vi_normal_prompt_indicator: None, default_multiline_indicator: None, render_right_prompt_on_last_line: false, + engine_state, + stack, } } @@ -106,7 +123,17 @@ impl Prompt for NushellPrompt { .to_string() .replace('\n', "\r\n"); - if self.shell_integration { + if self.shell_integration_osc633 { + if self.stack.get_env_var(&self.engine_state, "TERM_PROGRAM") + == Some(Value::test_string("vscode")) + { + // We're in vscode and we have osc633 enabled + format!("{VSCODE_PRE_PROMPT_MARKER}{prompt}{VSCODE_POST_PROMPT_MARKER}").into() + } else { + // If we're in VSCode but we don't find the env var, just return the regular markers + format!("{PRE_PROMPT_MARKER}{prompt}{POST_PROMPT_MARKER}").into() + } + } else if self.shell_integration_osc133 { format!("{PRE_PROMPT_MARKER}{prompt}{POST_PROMPT_MARKER}").into() } else { prompt.into() diff --git a/crates/nu-cli/src/prompt_update.rs b/crates/nu-cli/src/prompt_update.rs index 4c7cb7fcfe..0c5641378b 100644 --- a/crates/nu-cli/src/prompt_update.rs +++ b/crates/nu-cli/src/prompt_update.rs @@ -23,10 +23,37 @@ pub(crate) const TRANSIENT_PROMPT_INDICATOR_VI_NORMAL: &str = "TRANSIENT_PROMPT_INDICATOR_VI_NORMAL"; pub(crate) const TRANSIENT_PROMPT_MULTILINE_INDICATOR: &str = "TRANSIENT_PROMPT_MULTILINE_INDICATOR"; + +// Store all these Ansi Escape Markers here so they can be reused easily // According to Daniel Imms @Tyriar, we need to do these this way: // <133 A><133 B><133 C> pub(crate) const PRE_PROMPT_MARKER: &str = "\x1b]133;A\x1b\\"; pub(crate) const POST_PROMPT_MARKER: &str = "\x1b]133;B\x1b\\"; +pub(crate) const PRE_EXECUTION_MARKER: &str = "\x1b]133;C\x1b\\"; +#[allow(dead_code)] +pub(crate) const POST_EXECUTION_MARKER_PREFIX: &str = "\x1b]133;D;"; +#[allow(dead_code)] +pub(crate) const POST_EXECUTION_MARKER_SUFFIX: &str = "\x1b\\"; + +// OSC633 is the same as OSC133 but specifically for VSCode +pub(crate) const VSCODE_PRE_PROMPT_MARKER: &str = "\x1b]633;A\x1b\\"; +pub(crate) const VSCODE_POST_PROMPT_MARKER: &str = "\x1b]633;B\x1b\\"; +#[allow(dead_code)] +pub(crate) const VSCODE_PRE_EXECUTION_MARKER: &str = "\x1b]633;C\x1b\\"; +#[allow(dead_code)] +//"\x1b]633;D;{}\x1b\\" +pub(crate) const VSCODE_POST_EXECUTION_MARKER_PREFIX: &str = "\x1b]633;D;"; +#[allow(dead_code)] +pub(crate) const VSCODE_POST_EXECUTION_MARKER_SUFFIX: &str = "\x1b\\"; +#[allow(dead_code)] +pub(crate) const VSCODE_COMMANDLINE_MARKER: &str = "\x1b]633;E\x1b\\"; +#[allow(dead_code)] +// "\x1b]633;P;Cwd={}\x1b\\" +pub(crate) const VSCODE_CWD_PROPERTY_MARKER_PREFIX: &str = "\x1b]633;P;Cwd="; +#[allow(dead_code)] +pub(crate) const VSCODE_CWD_PROPERTY_MARKER_SUFFIX: &str = "\x1b\\"; + +pub(crate) const RESET_APPLICATION_MODE: &str = "\x1b[?1l"; fn get_prompt_string( prompt: &str, @@ -85,16 +112,46 @@ pub(crate) fn update_prompt( // Now that we have the prompt string lets ansify it. // <133 A><133 B><133 C> - let left_prompt_string = if config.shell_integration { - if let Some(prompt_string) = left_prompt_string { + let left_prompt_string_133 = if config.shell_integration_osc133 { + if let Some(prompt_string) = left_prompt_string.clone() { Some(format!( "{PRE_PROMPT_MARKER}{prompt_string}{POST_PROMPT_MARKER}" )) } else { - left_prompt_string + left_prompt_string.clone() } } else { - left_prompt_string + left_prompt_string.clone() + }; + + let left_prompt_string_633 = if config.shell_integration_osc633 { + if let Some(prompt_string) = left_prompt_string.clone() { + if stack.get_env_var(engine_state, "TERM_PROGRAM") == Some(Value::test_string("vscode")) + { + // If the user enabled osc633 and we're in vscode, use the vscode markers + Some(format!( + "{VSCODE_PRE_PROMPT_MARKER}{prompt_string}{VSCODE_POST_PROMPT_MARKER}" + )) + } else { + // otherwise, use the regular osc133 markers + Some(format!( + "{PRE_PROMPT_MARKER}{prompt_string}{POST_PROMPT_MARKER}" + )) + } + } else { + left_prompt_string.clone() + } + } else { + left_prompt_string.clone() + }; + + let left_prompt_string = match (left_prompt_string_133, left_prompt_string_633) { + (None, None) => left_prompt_string, + (None, Some(l633)) => Some(l633), + (Some(l133), None) => Some(l133), + // If both are set, it means we're in vscode, so use the vscode markers + // and even if we're not actually in vscode atm, the regular 133 markers are used + (Some(_l133), Some(l633)) => Some(l633), }; let right_prompt_string = get_prompt_string(PROMPT_COMMAND_RIGHT, config, engine_state, stack); diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 559611e265..0e5e24d46b 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -1,3 +1,9 @@ +use crate::prompt_update::{ + POST_EXECUTION_MARKER_PREFIX, POST_EXECUTION_MARKER_SUFFIX, PRE_EXECUTION_MARKER, + RESET_APPLICATION_MODE, VSCODE_CWD_PROPERTY_MARKER_PREFIX, VSCODE_CWD_PROPERTY_MARKER_SUFFIX, + VSCODE_POST_EXECUTION_MARKER_PREFIX, VSCODE_POST_EXECUTION_MARKER_SUFFIX, + VSCODE_PRE_EXECUTION_MARKER, +}; use crate::{ completions::NuCompleter, nu_highlight::NoOpHighlighter, @@ -42,16 +48,6 @@ use std::{ }; use sysinfo::System; -// According to Daniel Imms @Tyriar, we need to do these this way: -// <133 A><133 B><133 C> -// These first two have been moved to prompt_update to get as close as possible to the prompt. -// const PRE_PROMPT_MARKER: &str = "\x1b]133;A\x1b\\"; -// const POST_PROMPT_MARKER: &str = "\x1b]133;B\x1b\\"; -const PRE_EXECUTE_MARKER: &str = "\x1b]133;C\x1b\\"; -// This one is in get_command_finished_marker() now so we can capture the exit codes properly. -// const CMD_FINISHED_MARKER: &str = "\x1b]133;D;{}\x1b\\"; -const RESET_APPLICATION_MODE: &str = "\x1b[?1l"; - /// The main REPL loop, including spinning up the prompt itself. pub fn evaluate_repl( engine_state: &mut EngineState, @@ -66,7 +62,7 @@ pub fn evaluate_repl( // so that it may be read by various reedline plugins. During this, we // can't modify the stack, but at the end of the loop we take back ownership // from the Arc. This lets us avoid copying stack variables needlessly - let mut unique_stack = stack; + let mut unique_stack = stack.clone(); let config = engine_state.get_config(); let use_color = config.use_ansi_coloring; @@ -74,8 +70,19 @@ pub fn evaluate_repl( let mut entry_num = 0; - let shell_integration = config.shell_integration; - let nu_prompt = NushellPrompt::new(shell_integration); + // Let's grab the shell_integration configs + let shell_integration_osc2 = config.shell_integration_osc2; + let shell_integration_osc7 = config.shell_integration_osc7; + let shell_integration_osc9_9 = config.shell_integration_osc9_9; + let shell_integration_osc133 = config.shell_integration_osc133; + let shell_integration_osc633 = config.shell_integration_osc633; + + let nu_prompt = NushellPrompt::new( + shell_integration_osc133, + shell_integration_osc633, + engine_state.clone(), + stack.clone(), + ); let start_time = std::time::Instant::now(); // Translate environment variables from Strings to Values @@ -116,8 +123,22 @@ pub fn evaluate_repl( } let hostname = System::host_name(); - if shell_integration { - shell_integration_osc_7_633_2(hostname.as_deref(), engine_state, &mut unique_stack); + if shell_integration_osc2 { + run_shell_integration_osc2(None, engine_state, &mut unique_stack, use_color); + } + if shell_integration_osc7 { + run_shell_integration_osc7( + hostname.as_deref(), + engine_state, + &mut unique_stack, + use_color, + ); + } + if shell_integration_osc9_9 { + run_shell_integration_osc9_9(engine_state, &mut unique_stack, use_color); + } + if shell_integration_osc633 { + run_shell_integration_osc633(engine_state, &mut unique_stack, use_color); } engine_state.set_startup_time(entire_start_time.elapsed().as_nanos() as i64); @@ -513,7 +534,14 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { .with_highlighter(Box::::default()) // CLEAR STACK-REFERENCE 2 .with_completer(Box::::default()); - let shell_integration = config.shell_integration; + + // Let's grab the shell_integration configs + let shell_integration_osc2 = config.shell_integration_osc2; + let shell_integration_osc7 = config.shell_integration_osc7; + let shell_integration_osc9_9 = config.shell_integration_osc9_9; + let shell_integration_osc133 = config.shell_integration_osc133; + let shell_integration_osc633 = config.shell_integration_osc633; + let shell_integration_reset_application_mode = config.shell_integration_reset_application_mode; let mut stack = Stack::unwrap_unique(stack_arc); @@ -575,10 +603,40 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { repl.buffer = line_editor.current_buffer_contents().to_string(); drop(repl); - if shell_integration { + if shell_integration_osc633 { + if stack.get_env_var(engine_state, "TERM_PROGRAM") + == Some(Value::test_string("vscode")) + { + start_time = Instant::now(); + + run_ansi_sequence(VSCODE_PRE_EXECUTION_MARKER); + + perf( + "pre_execute_marker (633;C) ansi escape sequence", + start_time, + file!(), + line!(), + column!(), + use_color, + ); + } else { + start_time = Instant::now(); + + run_ansi_sequence(PRE_EXECUTION_MARKER); + + perf( + "pre_execute_marker (133;C) ansi escape sequence", + start_time, + file!(), + line!(), + column!(), + use_color, + ); + } + } else if shell_integration_osc133 { start_time = Instant::now(); - run_ansi_sequence(PRE_EXECUTE_MARKER); + run_ansi_sequence(PRE_EXECUTION_MARKER); perf( "pre_execute_marker (133;C) ansi escape sequence", @@ -598,20 +656,13 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { ReplOperation::AutoCd { cwd, target, span } => { do_auto_cd(target, cwd, &mut stack, engine_state, span); - if shell_integration { - start_time = Instant::now(); - - run_ansi_sequence(&get_command_finished_marker(&stack, engine_state)); - - perf( - "post_execute_marker (133;D) ansi escape sequences", - start_time, - file!(), - line!(), - column!(), - use_color, - ); - } + run_finaliziation_ansi_sequence( + &stack, + engine_state, + shell_integration_osc633, + shell_integration_osc133, + use_color, + ); } ReplOperation::RunCommand(cmd) => { line_editor = do_run_cmd( @@ -619,25 +670,18 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { &mut stack, engine_state, line_editor, - shell_integration, + shell_integration_osc2, *entry_num, use_color, ); - if shell_integration { - start_time = Instant::now(); - - run_ansi_sequence(&get_command_finished_marker(&stack, engine_state)); - - perf( - "post_execute_marker (133;D) ansi escape sequences", - start_time, - file!(), - line!(), - column!(), - use_color, - ); - } + run_finaliziation_ansi_sequence( + &stack, + engine_state, + shell_integration_osc633, + shell_integration_osc133, + use_color, + ); } // as the name implies, we do nothing in this case ReplOperation::DoNothing => {} @@ -663,56 +707,45 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { } } - if shell_integration { - start_time = Instant::now(); - - shell_integration_osc_7_633_2(hostname, engine_state, &mut stack); - - perf( - "shell_integration_finalize ansi escape sequences", - start_time, - file!(), - line!(), - column!(), - use_color, - ); + if shell_integration_osc2 { + run_shell_integration_osc2(None, engine_state, &mut stack, use_color); + } + if shell_integration_osc7 { + run_shell_integration_osc7(hostname, engine_state, &mut stack, use_color); + } + if shell_integration_osc9_9 { + run_shell_integration_osc9_9(engine_state, &mut stack, use_color); + } + if shell_integration_osc633 { + run_shell_integration_osc633(engine_state, &mut stack, use_color); + } + if shell_integration_reset_application_mode { + run_shell_integration_reset_application_mode(); } flush_engine_state_repl_buffer(engine_state, &mut line_editor); } Ok(Signal::CtrlC) => { // `Reedline` clears the line content. New prompt is shown - if shell_integration { - start_time = Instant::now(); - - run_ansi_sequence(&get_command_finished_marker(&stack, engine_state)); - - perf( - "command_finished_marker ansi escape sequence", - start_time, - file!(), - line!(), - column!(), - use_color, - ); - } + run_finaliziation_ansi_sequence( + &stack, + engine_state, + shell_integration_osc633, + shell_integration_osc133, + use_color, + ); } Ok(Signal::CtrlD) => { // When exiting clear to a new line - if shell_integration { - start_time = Instant::now(); - run_ansi_sequence(&get_command_finished_marker(&stack, engine_state)); + run_finaliziation_ansi_sequence( + &stack, + engine_state, + shell_integration_osc633, + shell_integration_osc133, + use_color, + ); - perf( - "command_finished_marker ansi escape sequence", - start_time, - file!(), - line!(), - column!(), - use_color, - ); - } println!(); return (false, stack, line_editor); } @@ -725,20 +758,14 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { // e.g. https://github.com/nushell/nushell/issues/6452 // Alternatively only allow that expected failures let the REPL loop } - if shell_integration { - start_time = Instant::now(); - run_ansi_sequence(&get_command_finished_marker(&stack, engine_state)); - - perf( - "command_finished_marker ansi escape sequence", - start_time, - file!(), - line!(), - column!(), - use_color, - ); - } + run_finaliziation_ansi_sequence( + &stack, + engine_state, + shell_integration_osc633, + shell_integration_osc133, + use_color, + ); } } perf( @@ -946,7 +973,7 @@ fn do_run_cmd( // we pass in the line editor so it can be dropped in the case of a process exit // (in the normal case we don't want to drop it so return it as-is otherwise) line_editor: Reedline, - shell_integration: bool, + shell_integration_osc2: bool, entry_num: usize, use_color: bool, ) -> Reedline { @@ -973,39 +1000,8 @@ fn do_run_cmd( } } - if shell_integration { - let start_time = Instant::now(); - if let Some(cwd) = stack.get_env_var(engine_state, "PWD") { - match cwd.coerce_into_string() { - Ok(path) => { - // Try to abbreviate string for windows title - let maybe_abbrev_path = if let Some(p) = nu_path::home_dir() { - path.replace(&p.as_path().display().to_string(), "~") - } else { - path - }; - let binary_name = s.split_whitespace().next(); - - if let Some(binary_name) = binary_name { - run_ansi_sequence(&format!( - "\x1b]2;{maybe_abbrev_path}> {binary_name}\x07" - )); - } - } - Err(e) => { - warn!("Could not coerce working directory to string {e}"); - } - } - } - - perf( - "set title with command ansi escape sequence", - start_time, - file!(), - line!(), - column!(), - use_color, - ); + if shell_integration_osc2 { + run_shell_integration_osc2(Some(s), engine_state, stack, use_color); } eval_source( @@ -1025,34 +1021,16 @@ fn do_run_cmd( /// can have more information about what is going on (both on startup and after we have /// run a command) /// -fn shell_integration_osc_7_633_2( - hostname: Option<&str>, +fn run_shell_integration_osc2( + command_name: Option<&str>, engine_state: &EngineState, stack: &mut Stack, + use_color: bool, ) { if let Some(cwd) = stack.get_env_var(engine_state, "PWD") { match cwd.coerce_into_string() { Ok(path) => { - // Supported escape sequences of Microsoft's Visual Studio Code (vscode) - // https://code.visualstudio.com/docs/terminal/shell-integration#_supported-escape-sequences - if stack.get_env_var(engine_state, "TERM_PROGRAM") - == Some(Value::test_string("vscode")) - { - // If we're in vscode, run their specific ansi escape sequence. - // This is helpful for ctrl+g to change directories in the terminal. - run_ansi_sequence(&format!("\x1b]633;P;Cwd={}\x1b\\", path)); - } else { - // Otherwise, communicate the path as OSC 7 (often used for spawning new tabs in the same dir) - run_ansi_sequence(&format!( - "\x1b]7;file://{}{}{}\x1b\\", - percent_encoding::utf8_percent_encode( - hostname.unwrap_or("localhost"), - percent_encoding::CONTROLS - ), - if path.starts_with('/') { "" } else { "/" }, - percent_encoding::utf8_percent_encode(&path, percent_encoding::CONTROLS) - )); - } + let start_time = Instant::now(); // Try to abbreviate string for windows title let maybe_abbrev_path = if let Some(p) = nu_path::home_dir() { @@ -1061,18 +1039,144 @@ fn shell_integration_osc_7_633_2( path }; + let title = match command_name { + Some(binary_name) => { + let split_binary_name = binary_name.split_whitespace().next(); + if let Some(binary_name) = split_binary_name { + format!("{maybe_abbrev_path}> {binary_name}") + } else { + maybe_abbrev_path.to_string() + } + } + None => maybe_abbrev_path.to_string(), + }; + // Set window title too // https://tldp.org/HOWTO/Xterm-Title-3.html // ESC]0;stringBEL -- Set icon name and window title to string // ESC]1;stringBEL -- Set icon name to string // ESC]2;stringBEL -- Set window title to string - run_ansi_sequence(&format!("\x1b]2;{maybe_abbrev_path}\x07")); + run_ansi_sequence(&format!("\x1b]2;{title}\x07")); + + perf( + "set title with command osc2", + start_time, + file!(), + line!(), + column!(), + use_color, + ); } Err(e) => { warn!("Could not coerce working directory to string {e}"); } } } +} + +fn run_shell_integration_osc7( + hostname: Option<&str>, + engine_state: &EngineState, + stack: &mut Stack, + use_color: bool, +) { + if let Some(cwd) = stack.get_env_var(engine_state, "PWD") { + match cwd.coerce_into_string() { + Ok(path) => { + let start_time = Instant::now(); + + // Otherwise, communicate the path as OSC 7 (often used for spawning new tabs in the same dir) + run_ansi_sequence(&format!( + "\x1b]7;file://{}{}{}\x1b\\", + percent_encoding::utf8_percent_encode( + hostname.unwrap_or("localhost"), + percent_encoding::CONTROLS + ), + if path.starts_with('/') { "" } else { "/" }, + percent_encoding::utf8_percent_encode(&path, percent_encoding::CONTROLS) + )); + + perf( + "communicate path to terminal with osc7", + start_time, + file!(), + line!(), + column!(), + use_color, + ); + } + Err(e) => { + warn!("Could not coerce working directory to string {e}"); + } + } + } +} + +fn run_shell_integration_osc9_9(engine_state: &EngineState, stack: &mut Stack, use_color: bool) { + if let Some(cwd) = stack.get_env_var(engine_state, "PWD") { + match cwd.coerce_into_string() { + Ok(path) => { + let start_time = Instant::now(); + + // Otherwise, communicate the path as OSC 9;9 from ConEmu (often used for spawning new tabs in the same dir) + run_ansi_sequence(&format!( + "\x1b]9;9;{}{}\x1b\\", + if path.starts_with('/') { "" } else { "/" }, + percent_encoding::utf8_percent_encode(&path, percent_encoding::CONTROLS) + )); + + perf( + "communicate path to terminal with osc9;9", + start_time, + file!(), + line!(), + column!(), + use_color, + ); + } + Err(e) => { + warn!("Could not coerce working directory to string {e}"); + } + } + } +} + +fn run_shell_integration_osc633(engine_state: &EngineState, stack: &mut Stack, use_color: bool) { + if let Some(cwd) = stack.get_env_var(engine_state, "PWD") { + match cwd.coerce_into_string() { + Ok(path) => { + // Supported escape sequences of Microsoft's Visual Studio Code (vscode) + // https://code.visualstudio.com/docs/terminal/shell-integration#_supported-escape-sequences + if stack.get_env_var(engine_state, "TERM_PROGRAM") + == Some(Value::test_string("vscode")) + { + let start_time = Instant::now(); + + // If we're in vscode, run their specific ansi escape sequence. + // This is helpful for ctrl+g to change directories in the terminal. + run_ansi_sequence(&format!( + "{}{}{}", + VSCODE_CWD_PROPERTY_MARKER_PREFIX, path, VSCODE_CWD_PROPERTY_MARKER_SUFFIX + )); + + perf( + "communicate path to terminal with osc633;P", + start_time, + file!(), + line!(), + column!(), + use_color, + ); + } + } + Err(e) => { + warn!("Could not coerce working directory to string {e}"); + } + } + } +} + +fn run_shell_integration_reset_application_mode() { run_ansi_sequence(RESET_APPLICATION_MODE); } @@ -1219,12 +1323,28 @@ fn map_nucursorshape_to_cursorshape(shape: NuCursorShape) -> Option String { +fn get_command_finished_marker(stack: &Stack, engine_state: &EngineState, vscode: bool) -> String { let exit_code = stack .get_env_var(engine_state, "LAST_EXIT_CODE") .and_then(|e| e.as_i64().ok()); - format!("\x1b]133;D;{}\x1b\\", exit_code.unwrap_or(0)) + if vscode { + // format!("\x1b]633;D;{}\x1b\\", exit_code.unwrap_or(0)) + format!( + "{}{}{}", + VSCODE_POST_EXECUTION_MARKER_PREFIX, + exit_code.unwrap_or(0), + VSCODE_POST_EXECUTION_MARKER_SUFFIX + ) + } else { + // format!("\x1b]133;D;{}\x1b\\", exit_code.unwrap_or(0)) + format!( + "{}{}{}", + POST_EXECUTION_MARKER_PREFIX, + exit_code.unwrap_or(0), + POST_EXECUTION_MARKER_SUFFIX + ) + } } fn run_ansi_sequence(seq: &str) { @@ -1235,6 +1355,58 @@ fn run_ansi_sequence(seq: &str) { } } +fn run_finaliziation_ansi_sequence( + stack: &Stack, + engine_state: &EngineState, + use_color: bool, + shell_integration_osc633: bool, + shell_integration_osc133: bool, +) { + if shell_integration_osc633 { + // Only run osc633 if we are in vscode + if stack.get_env_var(engine_state, "TERM_PROGRAM") == Some(Value::test_string("vscode")) { + let start_time = Instant::now(); + + run_ansi_sequence(&get_command_finished_marker(stack, engine_state, true)); + + perf( + "post_execute_marker (633;D) ansi escape sequences", + start_time, + file!(), + line!(), + column!(), + use_color, + ); + } else { + let start_time = Instant::now(); + + run_ansi_sequence(&get_command_finished_marker(stack, engine_state, false)); + + perf( + "post_execute_marker (133;D) ansi escape sequences", + start_time, + file!(), + line!(), + column!(), + use_color, + ); + } + } else if shell_integration_osc133 { + let start_time = Instant::now(); + + run_ansi_sequence(&get_command_finished_marker(stack, engine_state, false)); + + perf( + "post_execute_marker (133;D) ansi escape sequences", + start_time, + file!(), + line!(), + column!(), + use_color, + ); + } +} + // Absolute paths with a drive letter, like 'C:', 'D:\', 'E:\foo' #[cfg(windows)] static DRIVE_PATH_REGEX: once_cell::sync::Lazy = diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs index b032637275..11731e61b9 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -942,7 +942,11 @@ fn render_path_name( // clickable links don't work in remote SSH sessions let in_ssh_session = std::env::var("SSH_CLIENT").is_ok(); - let show_clickable_links = config.show_clickable_links_in_ls && !in_ssh_session && has_metadata; + //TODO: Deprecated show_clickable_links_in_ls in favor of shell_integration_osc8 + let show_clickable_links = config.show_clickable_links_in_ls + && !in_ssh_session + && has_metadata + && config.shell_integration_osc8; let ansi_style = style.map(Style::to_nu_ansi_term_style).unwrap_or_default(); diff --git a/crates/nu-protocol/src/config/mod.rs b/crates/nu-protocol/src/config/mod.rs index 312676038c..cd5ea69d7c 100644 --- a/crates/nu-protocol/src/config/mod.rs +++ b/crates/nu-protocol/src/config/mod.rs @@ -74,7 +74,14 @@ pub struct Config { pub menus: Vec, pub hooks: Hooks, pub rm_always_trash: bool, - pub shell_integration: bool, + // Shell integration OSC meaning is described in the default_config.nu + pub shell_integration_osc2: bool, + pub shell_integration_osc7: bool, + pub shell_integration_osc8: bool, + pub shell_integration_osc9_9: bool, + pub shell_integration_osc133: bool, + pub shell_integration_osc633: bool, + pub shell_integration_reset_application_mode: bool, pub buffer_editor: Value, pub table_index_mode: TableIndexMode, pub case_sensitive_completions: bool, @@ -154,7 +161,15 @@ impl Default for Config { use_ansi_coloring: true, bracketed_paste: true, edit_mode: EditBindings::default(), - shell_integration: false, + // shell_integration: false, + shell_integration_osc2: false, + shell_integration_osc7: false, + shell_integration_osc8: false, + shell_integration_osc9_9: false, + shell_integration_osc133: false, + shell_integration_osc633: false, + shell_integration_reset_application_mode: false, + render_right_prompt_on_last_line: false, hooks: Hooks::new(), @@ -639,7 +654,54 @@ impl Value { &mut errors); } "shell_integration" => { - process_bool_config(value, &mut errors, &mut config.shell_integration); + if let Value::Record { val, .. } = value { + val.to_mut().retain_mut(|key2, value| { + let span = value.span(); + match key2 { + "osc2" => { + process_bool_config(value, &mut errors, &mut config.shell_integration_osc2); + } + "osc7" => { + process_bool_config(value, &mut errors, &mut config.shell_integration_osc7); + } + "osc8" => { + process_bool_config(value, &mut errors, &mut config.shell_integration_osc8); + } + "osc9_9" => { + process_bool_config(value, &mut errors, &mut config.shell_integration_osc9_9); + } + "osc133" => { + process_bool_config(value, &mut errors, &mut config.shell_integration_osc133); + } + "osc633" => { + process_bool_config(value, &mut errors, &mut config.shell_integration_osc633); + } + "reset_application_mode" => { + process_bool_config(value, &mut errors, &mut config.shell_integration_reset_application_mode); + } + _ => { + report_invalid_key(&[key, key2], span, &mut errors); + return false; + } + }; + true + }) + } else { + report_invalid_value("boolean value is deprecated, should be a record. see default_conifg.nu.", span, &mut errors); + // Reconstruct + *value = Value::record( + record! { + "osc2" => Value::bool(config.shell_integration_osc2, span), + "ocs7" => Value::bool(config.shell_integration_osc7, span), + "osc8" => Value::bool(config.shell_integration_osc8, span), + "osc9_9" => Value::bool(config.shell_integration_osc9_9, span), + "osc133" => Value::bool(config.shell_integration_osc133, span), + "osc633" => Value::bool(config.shell_integration_osc633, span), + "reset_application_mode" => Value::bool(config.shell_integration_reset_application_mode, span), + }, + span, + ); + } } "buffer_editor" => match value { Value::Nothing { .. } | Value::String { .. } => { diff --git a/crates/nu-utils/src/sample_config/default_config.nu b/crates/nu-utils/src/sample_config/default_config.nu index 70e8a2ca7d..5681fb1a6d 100644 --- a/crates/nu-utils/src/sample_config/default_config.nu +++ b/crates/nu-utils/src/sample_config/default_config.nu @@ -236,7 +236,34 @@ $env.config = { use_ansi_coloring: true bracketed_paste: true # enable bracketed paste, currently useless on windows edit_mode: emacs # emacs, vi - shell_integration: false # enables terminal shell integration. Off by default, as some terminals have issues with this. + shell_integration: { + # osc2 abbreviates the path if in the home_dir, sets the tab/window title, shows the running command in the tab/window title + osc2: true + # osc7 is a way to communicate the path to the terminal, this is helpful for spawning new tabs in the same directory + osc7: true + # osc8 is also implemented as the deprecated setting ls.show_clickable_links, it shows clickable links in ls output if your terminal supports it. show_clickable_links is deprecated in favor of osc8 + osc8: true + # osc9_9 is from ConEmu and is starting to get wider support. It's similar to osc7 in that it communicates the path to the terminal + osc9_9: false + # osc133 is several escapes invented by Final Term which include the supported ones below. + # 133;A - Mark prompt start + # 133;B - Mark prompt end + # 133;C - Mark pre-execution + # 133;D;exit - Mark execution finished with exit code + # This is used to enable terminals to know where the prompt is, the command is, where the command finishes, and where the output of the command is + osc133: true + # osc633 is closely related to osc133 but only exists in visual studio code (vscode) and supports their shell integration features + # 633;A - Mark prompt start + # 633;B - Mark prompt end + # 633;C - Mark pre-execution + # 633;D;exit - Mark execution finished with exit code + # 633;E - NOT IMPLEMENTED - Explicitly set the command line with an optional nonce + # 633;P;Cwd= - Mark the current working directory and communicate it to the terminal + # and also helps with the run recent menu in vscode + osc633: true + # reset_application_mode is escape \x1b[?1l and was added to help ssh work better + reset_application_mode: true + } render_right_prompt_on_last_line: false # true or false to enable or disable right prompt to be rendered on last line of the prompt. use_kitty_protocol: false # enables keyboard enhancement protocol implemented by kitty console, only if your terminal support this. highlight_resolved_externals: false # true enables highlighting of external commands in the repl resolved by which. From b88d8726d0d448a75184aa403c1af1f0e5e9125c Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Thu, 2 May 2024 19:29:03 +0200 Subject: [PATCH 05/16] Rework for new clippy lints (#12736) - **Clippy lint `assigning_clones`** - **Clippy lint `legacy_numeric_constants`** - **`clippy::float_equality_without_abs`** - **`nu-table`: clippy::zero_repeat_side_effects** --------- Co-authored-by: Ian Manske --- crates/nu-cmd-lang/src/core_commands/do_.rs | 4 +- crates/nu-command/src/bytes/at.rs | 2 +- .../nu-command/src/strings/str_/substring.rs | 4 +- crates/nu-json/src/value.rs | 8 ++-- crates/nu-protocol/src/lev_distance.rs | 2 +- .../src/plugin/registry_file/mod.rs | 2 +- crates/nu-protocol/src/value/mod.rs | 22 +++++----- crates/nu-table/tests/style.rs | 44 +++++-------------- 8 files changed, 31 insertions(+), 57 deletions(-) diff --git a/crates/nu-cmd-lang/src/core_commands/do_.rs b/crates/nu-cmd-lang/src/core_commands/do_.rs index 36fb13cf10..c2b630e08d 100644 --- a/crates/nu-cmd-lang/src/core_commands/do_.rs +++ b/crates/nu-cmd-lang/src/core_commands/do_.rs @@ -127,7 +127,7 @@ impl Command for Do { let stderr_msg = match stderr { None => "".to_string(), Some(stderr_stream) => { - stderr_ctrlc = stderr_stream.ctrlc.clone(); + stderr_ctrlc.clone_from(&stderr_stream.ctrlc); stderr_stream.into_string().map(|s| s.item)? } }; @@ -152,7 +152,7 @@ impl Command for Do { let exit_code: Vec = match exit_code { None => vec![], Some(exit_code_stream) => { - exit_code_ctrlc = exit_code_stream.ctrlc.clone(); + exit_code_ctrlc.clone_from(&exit_code_stream.ctrlc); exit_code_stream.into_iter().collect() } }; diff --git a/crates/nu-command/src/bytes/at.rs b/crates/nu-command/src/bytes/at.rs index 55b2998ec4..e90312603f 100644 --- a/crates/nu-command/src/bytes/at.rs +++ b/crates/nu-command/src/bytes/at.rs @@ -150,7 +150,7 @@ fn action(input: &Value, args: &Arguments, head: Span) -> Value { head, ), Ordering::Less => Value::binary( - if end == isize::max_value() { + if end == isize::MAX { val.iter() .skip(start as usize) .copied() diff --git a/crates/nu-command/src/strings/str_/substring.rs b/crates/nu-command/src/strings/str_/substring.rs index d137ce5c76..55871401ef 100644 --- a/crates/nu-command/src/strings/str_/substring.rs +++ b/crates/nu-command/src/strings/str_/substring.rs @@ -149,7 +149,7 @@ fn action(input: &Value, args: &Arguments, head: Span) -> Value { ), Ordering::Less => Value::string( { - if end == isize::max_value() { + if end == isize::MAX { if args.graphemes { s.graphemes(true) .skip(start as usize) @@ -245,7 +245,7 @@ mod tests { expectation("andre", (0, -1)), // str substring [ -4 , _ ] // str substring -4 , - expectation("dres", (-4, isize::max_value())), + expectation("dres", (-4, isize::MAX)), expectation("", (0, -110)), expectation("", (6, 0)), expectation("", (6, -1)), diff --git a/crates/nu-json/src/value.rs b/crates/nu-json/src/value.rs index a37c531e59..88f878f260 100644 --- a/crates/nu-json/src/value.rs +++ b/crates/nu-json/src/value.rs @@ -1141,18 +1141,18 @@ mod test { let v: Value = from_str("{\"a\":1.1}").unwrap(); let vo = v.as_object().unwrap(); - assert!(vo["a"].as_f64().unwrap() - 1.1 < std::f64::EPSILON); + assert!((vo["a"].as_f64().unwrap() - 1.1).abs() < f64::EPSILON); let v: Value = from_str("{\"a\":-1.1}").unwrap(); let vo = v.as_object().unwrap(); - assert!(vo["a"].as_f64().unwrap() + 1.1 > -(std::f64::EPSILON)); + assert!((vo["a"].as_f64().unwrap() + 1.1).abs() < f64::EPSILON); let v: Value = from_str("{\"a\":1e6}").unwrap(); let vo = v.as_object().unwrap(); - assert!(vo["a"].as_f64().unwrap() - 1e6 < std::f64::EPSILON); + assert!((vo["a"].as_f64().unwrap() - 1e6).abs() < f64::EPSILON); let v: Value = from_str("{\"a\":-1e6}").unwrap(); let vo = v.as_object().unwrap(); - assert!(vo["a"].as_f64().unwrap() + 1e6 > -(std::f64::EPSILON)); + assert!((vo["a"].as_f64().unwrap() + 1e6).abs() < f64::EPSILON); } } diff --git a/crates/nu-protocol/src/lev_distance.rs b/crates/nu-protocol/src/lev_distance.rs index ced34e9bc5..51dc856d86 100644 --- a/crates/nu-protocol/src/lev_distance.rs +++ b/crates/nu-protocol/src/lev_distance.rs @@ -55,7 +55,7 @@ pub fn lev_distance(a: &str, b: &str, limit: usize) -> Option { /// Finds the Levenshtein distance between two strings. pub fn levenshtein_distance(a: &str, b: &str) -> usize { - lev_distance(a, b, usize::max_value()).unwrap_or(usize::max_value()) + lev_distance(a, b, usize::MAX).unwrap_or(usize::MAX) } /// Provides a word similarity score between two words that accounts for substrings being more /// meaningful than a typical Levenshtein distance. The lower the score, the closer the match. diff --git a/crates/nu-protocol/src/plugin/registry_file/mod.rs b/crates/nu-protocol/src/plugin/registry_file/mod.rs index d3eb4a9d02..0b5cc9c315 100644 --- a/crates/nu-protocol/src/plugin/registry_file/mod.rs +++ b/crates/nu-protocol/src/plugin/registry_file/mod.rs @@ -66,7 +66,7 @@ impl PluginRegistryFile { error_span: Option, ) -> Result<(), ShellError> { // Update the Nushell version before writing - self.nushell_version = env!("CARGO_PKG_VERSION").to_owned(); + env!("CARGO_PKG_VERSION").clone_into(&mut self.nushell_version); // Format is brotli compressed messagepack let mut brotli_writer = diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 2c5019a369..64b5abf64d 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -2893,7 +2893,7 @@ impl Value { if *rhs != 0 { Ok(Value::int( (*lhs as f64 / *rhs as f64) - .clamp(std::i64::MIN as f64, std::i64::MAX as f64) + .clamp(i64::MIN as f64, i64::MAX as f64) .floor() as i64, span, )) @@ -2905,7 +2905,7 @@ impl Value { if *rhs != 0.0 { Ok(Value::int( (*lhs as f64 / *rhs) - .clamp(std::i64::MIN as f64, std::i64::MAX as f64) + .clamp(i64::MIN as f64, i64::MAX as f64) .floor() as i64, span, )) @@ -2917,7 +2917,7 @@ impl Value { if *rhs != 0 { Ok(Value::int( (*lhs / *rhs as f64) - .clamp(std::i64::MIN as f64, std::i64::MAX as f64) + .clamp(i64::MIN as f64, i64::MAX as f64) .floor() as i64, span, )) @@ -2928,9 +2928,7 @@ impl Value { (Value::Float { val: lhs, .. }, Value::Float { val: rhs, .. }) => { if *rhs != 0.0 { Ok(Value::int( - (lhs / rhs) - .clamp(std::i64::MIN as f64, std::i64::MAX as f64) - .floor() as i64, + (lhs / rhs).clamp(i64::MIN as f64, i64::MAX as f64).floor() as i64, span, )) } else { @@ -2941,7 +2939,7 @@ impl Value { if *rhs != 0 { Ok(Value::int( (*lhs as f64 / *rhs as f64) - .clamp(std::i64::MIN as f64, std::i64::MAX as f64) + .clamp(i64::MIN as f64, i64::MAX as f64) .floor() as i64, span, )) @@ -2953,7 +2951,7 @@ impl Value { if *rhs != 0 { Ok(Value::filesize( ((*lhs as f64) / (*rhs as f64)) - .clamp(std::i64::MIN as f64, std::i64::MAX as f64) + .clamp(i64::MIN as f64, i64::MAX as f64) .floor() as i64, span, )) @@ -2965,7 +2963,7 @@ impl Value { if *rhs != 0.0 { Ok(Value::filesize( (*lhs as f64 / *rhs) - .clamp(std::i64::MIN as f64, std::i64::MAX as f64) + .clamp(i64::MIN as f64, i64::MAX as f64) .floor() as i64, span, )) @@ -2977,7 +2975,7 @@ impl Value { if *rhs != 0 { Ok(Value::int( (*lhs as f64 / *rhs as f64) - .clamp(std::i64::MIN as f64, std::i64::MAX as f64) + .clamp(i64::MIN as f64, i64::MAX as f64) .floor() as i64, span, )) @@ -2989,7 +2987,7 @@ impl Value { if *rhs != 0 { Ok(Value::duration( (*lhs as f64 / *rhs as f64) - .clamp(std::i64::MIN as f64, std::i64::MAX as f64) + .clamp(i64::MIN as f64, i64::MAX as f64) .floor() as i64, span, )) @@ -3001,7 +2999,7 @@ impl Value { if *rhs != 0.0 { Ok(Value::duration( (*lhs as f64 / *rhs) - .clamp(std::i64::MIN as f64, std::i64::MAX as f64) + .clamp(i64::MIN as f64, i64::MAX as f64) .floor() as i64, span, )) diff --git a/crates/nu-table/tests/style.rs b/crates/nu-table/tests/style.rs index 58b609b99e..032e5f9c45 100644 --- a/crates/nu-table/tests/style.rs +++ b/crates/nu-table/tests/style.rs @@ -47,10 +47,7 @@ fn test_rounded() { ╰───┴───┴───┴───╯" ); - assert_eq!( - create_table_with_size(vec![row(4); 0], true, theme::rounded()), - "" - ); + assert_eq!(create_table_with_size(vec![], true, theme::rounded()), ""); } #[test] @@ -98,10 +95,7 @@ fn test_basic() { +---+---+---+---+" ); - assert_eq!( - create_table_with_size(vec![row(4); 0], true, theme::basic()), - "" - ); + assert_eq!(create_table_with_size(vec![], true, theme::basic()), ""); } #[test] @@ -146,7 +140,7 @@ fn test_reinforced() { ); assert_eq!( - create_table_with_size(vec![row(4); 0], true, theme::reinforced()), + create_table_with_size(vec![], true, theme::reinforced()), "" ); } @@ -196,10 +190,7 @@ fn test_compact() { ) ); - assert_eq!( - create_table_with_size(vec![row(4); 0], true, theme::compact()), - "" - ); + assert_eq!(create_table_with_size(vec![], true, theme::compact()), ""); } #[test] @@ -248,7 +239,7 @@ fn test_compact_double() { ); assert_eq!( - create_table_with_size(vec![row(4); 0], true, theme::compact_double()), + create_table_with_size(vec![], true, theme::compact_double()), "" ); } @@ -296,10 +287,7 @@ fn test_heavy() { ┗━━━┻━━━┻━━━┻━━━┛" ); - assert_eq!( - create_table_with_size(vec![row(4); 0], true, theme::heavy()), - "" - ); + assert_eq!(create_table_with_size(vec![], true, theme::heavy()), ""); } #[test] @@ -334,10 +322,7 @@ fn test_light() { concat!(" 0 1 2 3 \n", " 0 1 2 3 ") ); - assert_eq!( - create_table_with_size(vec![row(4); 0], true, theme::light()), - "" - ); + assert_eq!(create_table_with_size(vec![], true, theme::light()), ""); } #[test] @@ -367,10 +352,7 @@ fn test_none() { concat!(" 0 1 2 3 \n", " 0 1 2 3 ") ); - assert_eq!( - create_table_with_size(vec![row(4); 0], true, theme::none()), - "" - ); + assert_eq!(create_table_with_size(vec![], true, theme::none()), ""); } #[test] @@ -418,10 +400,7 @@ fn test_thin() { └───┴───┴───┴───┘" ); - assert_eq!( - create_table_with_size(vec![row(4); 0], true, theme::thin()), - "" - ); + assert_eq!(create_table_with_size(vec![], true, theme::thin()), ""); } #[test] @@ -469,10 +448,7 @@ fn test_with_love() { ) ); - assert_eq!( - create_table_with_size(vec![row(4); 0], true, theme::with_love()), - "" - ); + assert_eq!(create_table_with_size(vec![], true, theme::with_love()), ""); } fn create_table(data: Vec>>, with_header: bool, theme: theme) -> String { From be6137d13652506476f663b69e58d6c135268470 Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Thu, 2 May 2024 19:31:51 +0200 Subject: [PATCH 06/16] Fix clippy::wrong_self_convention in polars plugin (#12737) Expected `into_` for `fn(self) -> T` --- crates/nu_plugin_polars/src/dataframe/eager/cast.rs | 2 +- crates/nu_plugin_polars/src/dataframe/eager/first.rs | 2 +- crates/nu_plugin_polars/src/dataframe/eager/last.rs | 2 +- .../src/dataframe/expressions/alias.rs | 2 +- .../src/dataframe/expressions/arg_where.rs | 2 +- .../src/dataframe/expressions/datepart.rs | 2 +- .../src/dataframe/expressions/expressions_macro.rs | 6 +++--- .../src/dataframe/expressions/is_in.rs | 2 +- .../src/dataframe/expressions/otherwise.rs | 4 ++-- .../src/dataframe/expressions/when.rs | 12 ++++++------ .../nu_plugin_polars/src/dataframe/lazy/explode.rs | 2 +- .../nu_plugin_polars/src/dataframe/lazy/fill_nan.rs | 4 ++-- .../nu_plugin_polars/src/dataframe/lazy/fill_null.rs | 6 +++--- crates/nu_plugin_polars/src/dataframe/lazy/filter.rs | 2 +- crates/nu_plugin_polars/src/dataframe/lazy/median.rs | 2 +- .../nu_plugin_polars/src/dataframe/lazy/quantile.rs | 2 +- .../src/dataframe/series/masks/is_not_null.rs | 2 +- .../src/dataframe/series/masks/is_null.rs | 2 +- .../src/dataframe/series/n_unique.rs | 2 +- .../nu_plugin_polars/src/dataframe/series/shift.rs | 2 +- .../src/dataframe/values/nu_expression/mod.rs | 4 ++-- .../src/dataframe/values/nu_lazyframe/mod.rs | 2 +- 22 files changed, 34 insertions(+), 34 deletions(-) diff --git a/crates/nu_plugin_polars/src/dataframe/eager/cast.rs b/crates/nu_plugin_polars/src/dataframe/eager/cast.rs index 23cbb0bc7c..074e162201 100644 --- a/crates/nu_plugin_polars/src/dataframe/eager/cast.rs +++ b/crates/nu_plugin_polars/src/dataframe/eager/cast.rs @@ -104,7 +104,7 @@ impl PluginCommand for CastDF { PolarsPluginObject::NuExpression(expr) => { let dtype: String = call.req(0)?; let dtype = str_to_dtype(&dtype, call.head)?; - let expr: NuExpression = expr.to_polars().cast(dtype).into(); + let expr: NuExpression = expr.into_polars().cast(dtype).into(); expr.to_pipeline_data(plugin, engine, call.head) } _ => Err(cant_convert_err( diff --git a/crates/nu_plugin_polars/src/dataframe/eager/first.rs b/crates/nu_plugin_polars/src/dataframe/eager/first.rs index 6bbdc66f75..69f80e42c0 100644 --- a/crates/nu_plugin_polars/src/dataframe/eager/first.rs +++ b/crates/nu_plugin_polars/src/dataframe/eager/first.rs @@ -103,7 +103,7 @@ impl PluginCommand for FirstDF { command(plugin, engine, call, df).map_err(|e| e.into()) } else { let expr = NuExpression::try_from_value(plugin, &value)?; - let expr: NuExpression = expr.to_polars().first().into(); + let expr: NuExpression = expr.into_polars().first().into(); expr.to_pipeline_data(plugin, engine, call.head) .map_err(LabeledError::from) diff --git a/crates/nu_plugin_polars/src/dataframe/eager/last.rs b/crates/nu_plugin_polars/src/dataframe/eager/last.rs index 840fe063e9..0da0832d9b 100644 --- a/crates/nu_plugin_polars/src/dataframe/eager/last.rs +++ b/crates/nu_plugin_polars/src/dataframe/eager/last.rs @@ -78,7 +78,7 @@ impl PluginCommand for LastDF { command(plugin, engine, call, df).map_err(|e| e.into()) } else { let expr = NuExpression::try_from_value(plugin, &value)?; - let expr: NuExpression = expr.to_polars().last().into(); + let expr: NuExpression = expr.into_polars().last().into(); expr.to_pipeline_data(plugin, engine, call.head) .map_err(LabeledError::from) diff --git a/crates/nu_plugin_polars/src/dataframe/expressions/alias.rs b/crates/nu_plugin_polars/src/dataframe/expressions/alias.rs index 93ed234443..c780f13d9f 100644 --- a/crates/nu_plugin_polars/src/dataframe/expressions/alias.rs +++ b/crates/nu_plugin_polars/src/dataframe/expressions/alias.rs @@ -67,7 +67,7 @@ impl PluginCommand for ExprAlias { let alias: String = call.req(0)?; let expr = NuExpression::try_from_pipeline(plugin, input, call.head)?; - let expr: NuExpression = expr.to_polars().alias(alias.as_str()).into(); + let expr: NuExpression = expr.into_polars().alias(alias.as_str()).into(); expr.to_pipeline_data(plugin, engine, call.head) .map_err(LabeledError::from) diff --git a/crates/nu_plugin_polars/src/dataframe/expressions/arg_where.rs b/crates/nu_plugin_polars/src/dataframe/expressions/arg_where.rs index 07ae483fc5..a9d29c3b14 100644 --- a/crates/nu_plugin_polars/src/dataframe/expressions/arg_where.rs +++ b/crates/nu_plugin_polars/src/dataframe/expressions/arg_where.rs @@ -62,7 +62,7 @@ impl PluginCommand for ExprArgWhere { ) -> Result { let value: Value = call.req(0)?; let expr = NuExpression::try_from_value(plugin, &value)?; - let expr: NuExpression = arg_where(expr.to_polars()).into(); + let expr: NuExpression = arg_where(expr.into_polars()).into(); expr.to_pipeline_data(plugin, engine, call.head) .map_err(LabeledError::from) } diff --git a/crates/nu_plugin_polars/src/dataframe/expressions/datepart.rs b/crates/nu_plugin_polars/src/dataframe/expressions/datepart.rs index 02fbb1bf34..1cbb5fa532 100644 --- a/crates/nu_plugin_polars/src/dataframe/expressions/datepart.rs +++ b/crates/nu_plugin_polars/src/dataframe/expressions/datepart.rs @@ -127,7 +127,7 @@ impl PluginCommand for ExprDatePart { let part: Spanned = call.req(0)?; let expr = NuExpression::try_from_pipeline(plugin, input, call.head)?; - let expr_dt = expr.to_polars().dt(); + let expr_dt = expr.into_polars().dt(); let expr: NuExpression = match part.item.as_str() { "year" => expr_dt.year(), "quarter" => expr_dt.quarter(), diff --git a/crates/nu_plugin_polars/src/dataframe/expressions/expressions_macro.rs b/crates/nu_plugin_polars/src/dataframe/expressions/expressions_macro.rs index fe79b9e28e..1a566f6989 100644 --- a/crates/nu_plugin_polars/src/dataframe/expressions/expressions_macro.rs +++ b/crates/nu_plugin_polars/src/dataframe/expressions/expressions_macro.rs @@ -50,7 +50,7 @@ macro_rules! expr_command { ) -> Result { let expr = NuExpression::try_from_pipeline(plugin, input, call.head) .map_err(LabeledError::from)?; - let expr: NuExpression = expr.to_polars().$func().into(); + let expr: NuExpression = expr.into_polars().$func().into(); expr.to_pipeline_data(plugin, engine, call.head) .map_err(LabeledError::from) } @@ -181,7 +181,7 @@ macro_rules! lazy_expr_command { } else { let expr = NuExpression::try_from_value(plugin, &value).map_err(LabeledError::from)?; - let expr: NuExpression = expr.to_polars().$func().into(); + let expr: NuExpression = expr.into_polars().$func().into(); expr.to_pipeline_data(plugin, engine, call.head) .map_err(LabeledError::from) } @@ -261,7 +261,7 @@ macro_rules! lazy_expr_command { .map_err(LabeledError::from) } else { let expr = NuExpression::try_from_value(plugin, &value)?; - let expr: NuExpression = expr.to_polars().$func($ddof).into(); + let expr: NuExpression = expr.into_polars().$func($ddof).into(); expr.to_pipeline_data(plugin, engine, call.head) .map_err(LabeledError::from) } diff --git a/crates/nu_plugin_polars/src/dataframe/expressions/is_in.rs b/crates/nu_plugin_polars/src/dataframe/expressions/is_in.rs index b931a17764..1f270837bb 100644 --- a/crates/nu_plugin_polars/src/dataframe/expressions/is_in.rs +++ b/crates/nu_plugin_polars/src/dataframe/expressions/is_in.rs @@ -153,7 +153,7 @@ fn command_expr( }); } - let expr: NuExpression = expr.to_polars().is_in(lit(list)).into(); + let expr: NuExpression = expr.into_polars().is_in(lit(list)).into(); expr.to_pipeline_data(plugin, engine, call.head) } diff --git a/crates/nu_plugin_polars/src/dataframe/expressions/otherwise.rs b/crates/nu_plugin_polars/src/dataframe/expressions/otherwise.rs index e2697f7167..c887789268 100644 --- a/crates/nu_plugin_polars/src/dataframe/expressions/otherwise.rs +++ b/crates/nu_plugin_polars/src/dataframe/expressions/otherwise.rs @@ -101,9 +101,9 @@ impl PluginCommand for ExprOtherwise { let value = input.into_value(call.head); let complete: NuExpression = match NuWhen::try_from_value(plugin, &value)?.when_type { - NuWhenType::Then(then) => then.otherwise(otherwise_predicate.to_polars()).into(), + NuWhenType::Then(then) => then.otherwise(otherwise_predicate.into_polars()).into(), NuWhenType::ChainedThen(chained_when) => chained_when - .otherwise(otherwise_predicate.to_polars()) + .otherwise(otherwise_predicate.into_polars()) .into(), }; complete diff --git a/crates/nu_plugin_polars/src/dataframe/expressions/when.rs b/crates/nu_plugin_polars/src/dataframe/expressions/when.rs index 5ea542619a..1639ed44be 100644 --- a/crates/nu_plugin_polars/src/dataframe/expressions/when.rs +++ b/crates/nu_plugin_polars/src/dataframe/expressions/when.rs @@ -113,17 +113,17 @@ impl PluginCommand for ExprWhen { let value = input.into_value(call.head); let when_then: NuWhen = match value { - Value::Nothing { .. } => when(when_predicate.to_polars()) - .then(then_predicate.to_polars()) + Value::Nothing { .. } => when(when_predicate.into_polars()) + .then(then_predicate.into_polars()) .into(), v => match NuWhen::try_from_value(plugin, &v)?.when_type { NuWhenType::Then(when_then) => when_then - .when(when_predicate.to_polars()) - .then(then_predicate.to_polars()) + .when(when_predicate.into_polars()) + .then(then_predicate.into_polars()) .into(), NuWhenType::ChainedThen(when_then_then) => when_then_then - .when(when_predicate.to_polars()) - .then(then_predicate.to_polars()) + .when(when_predicate.into_polars()) + .then(then_predicate.into_polars()) .into(), }, }; diff --git a/crates/nu_plugin_polars/src/dataframe/lazy/explode.rs b/crates/nu_plugin_polars/src/dataframe/lazy/explode.rs index b1fb562eb8..b0609d7a3c 100644 --- a/crates/nu_plugin_polars/src/dataframe/lazy/explode.rs +++ b/crates/nu_plugin_polars/src/dataframe/lazy/explode.rs @@ -160,7 +160,7 @@ pub(crate) fn explode_expr( call: &EvaluatedCall, expr: NuExpression, ) -> Result { - let expr: NuExpression = expr.to_polars().explode().into(); + let expr: NuExpression = expr.into_polars().explode().into(); expr.to_pipeline_data(plugin, engine, call.head) } diff --git a/crates/nu_plugin_polars/src/dataframe/lazy/fill_nan.rs b/crates/nu_plugin_polars/src/dataframe/lazy/fill_nan.rs index 7c5cdfe574..851be588f9 100644 --- a/crates/nu_plugin_polars/src/dataframe/lazy/fill_nan.rs +++ b/crates/nu_plugin_polars/src/dataframe/lazy/fill_nan.rs @@ -168,8 +168,8 @@ fn cmd_expr( expr: NuExpression, fill: Value, ) -> Result { - let fill = NuExpression::try_from_value(plugin, &fill)?.to_polars(); - let expr: NuExpression = expr.to_polars().fill_nan(fill).into(); + let fill = NuExpression::try_from_value(plugin, &fill)?.into_polars(); + let expr: NuExpression = expr.into_polars().fill_nan(fill).into(); expr.to_pipeline_data(plugin, engine, call.head) } diff --git a/crates/nu_plugin_polars/src/dataframe/lazy/fill_null.rs b/crates/nu_plugin_polars/src/dataframe/lazy/fill_null.rs index a70e2b72ab..e68a6829f0 100644 --- a/crates/nu_plugin_polars/src/dataframe/lazy/fill_null.rs +++ b/crates/nu_plugin_polars/src/dataframe/lazy/fill_null.rs @@ -95,7 +95,7 @@ fn cmd_lazy( lazy: NuLazyFrame, fill: Value, ) -> Result { - let expr = NuExpression::try_from_value(plugin, &fill)?.to_polars(); + let expr = NuExpression::try_from_value(plugin, &fill)?.into_polars(); let lazy = NuLazyFrame::new(lazy.from_eager, lazy.to_polars().fill_null(expr)); lazy.to_pipeline_data(plugin, engine, call.head) } @@ -107,8 +107,8 @@ fn cmd_expr( expr: NuExpression, fill: Value, ) -> Result { - let fill = NuExpression::try_from_value(plugin, &fill)?.to_polars(); - let expr: NuExpression = expr.to_polars().fill_null(fill).into(); + let fill = NuExpression::try_from_value(plugin, &fill)?.into_polars(); + let expr: NuExpression = expr.into_polars().fill_null(fill).into(); expr.to_pipeline_data(plugin, engine, call.head) } diff --git a/crates/nu_plugin_polars/src/dataframe/lazy/filter.rs b/crates/nu_plugin_polars/src/dataframe/lazy/filter.rs index 9e8d0aea8a..246976c2ee 100644 --- a/crates/nu_plugin_polars/src/dataframe/lazy/filter.rs +++ b/crates/nu_plugin_polars/src/dataframe/lazy/filter.rs @@ -87,7 +87,7 @@ fn command( ) -> Result { let lazy = NuLazyFrame::new( lazy.from_eager, - lazy.to_polars().filter(filter_expr.to_polars()), + lazy.to_polars().filter(filter_expr.into_polars()), ); lazy.to_pipeline_data(plugin, engine, call.head) } diff --git a/crates/nu_plugin_polars/src/dataframe/lazy/median.rs b/crates/nu_plugin_polars/src/dataframe/lazy/median.rs index c31177a5a2..c3cfdba099 100644 --- a/crates/nu_plugin_polars/src/dataframe/lazy/median.rs +++ b/crates/nu_plugin_polars/src/dataframe/lazy/median.rs @@ -94,7 +94,7 @@ impl PluginCommand for LazyMedian { PolarsPluginObject::NuDataFrame(df) => command(plugin, engine, call, df.lazy()), PolarsPluginObject::NuLazyFrame(lazy) => command(plugin, engine, call, lazy), PolarsPluginObject::NuExpression(expr) => { - let expr: NuExpression = expr.to_polars().median().into(); + let expr: NuExpression = expr.into_polars().median().into(); expr.to_pipeline_data(plugin, engine, call.head) } _ => Err(cant_convert_err( diff --git a/crates/nu_plugin_polars/src/dataframe/lazy/quantile.rs b/crates/nu_plugin_polars/src/dataframe/lazy/quantile.rs index ede9c905ed..d8cb3e4cb6 100644 --- a/crates/nu_plugin_polars/src/dataframe/lazy/quantile.rs +++ b/crates/nu_plugin_polars/src/dataframe/lazy/quantile.rs @@ -106,7 +106,7 @@ impl PluginCommand for LazyQuantile { PolarsPluginObject::NuLazyFrame(lazy) => command(plugin, engine, call, lazy, quantile), PolarsPluginObject::NuExpression(expr) => { let expr: NuExpression = expr - .to_polars() + .into_polars() .quantile(lit(quantile), QuantileInterpolOptions::default()) .into(); expr.to_pipeline_data(plugin, engine, call.head) diff --git a/crates/nu_plugin_polars/src/dataframe/series/masks/is_not_null.rs b/crates/nu_plugin_polars/src/dataframe/series/masks/is_not_null.rs index 8ded0329d6..218cd116b4 100644 --- a/crates/nu_plugin_polars/src/dataframe/series/masks/is_not_null.rs +++ b/crates/nu_plugin_polars/src/dataframe/series/masks/is_not_null.rs @@ -86,7 +86,7 @@ impl PluginCommand for IsNotNull { command(plugin, engine, call, lazy.collect(call.head)?) } PolarsPluginObject::NuExpression(expr) => { - let expr: NuExpression = expr.to_polars().is_not_null().into(); + let expr: NuExpression = expr.into_polars().is_not_null().into(); expr.to_pipeline_data(plugin, engine, call.head) } _ => Err(cant_convert_err( diff --git a/crates/nu_plugin_polars/src/dataframe/series/masks/is_null.rs b/crates/nu_plugin_polars/src/dataframe/series/masks/is_null.rs index b67e4e7702..beb3793661 100644 --- a/crates/nu_plugin_polars/src/dataframe/series/masks/is_null.rs +++ b/crates/nu_plugin_polars/src/dataframe/series/masks/is_null.rs @@ -88,7 +88,7 @@ impl PluginCommand for IsNull { command(plugin, engine, call, lazy.collect(call.head)?) } PolarsPluginObject::NuExpression(expr) => { - let expr: NuExpression = expr.to_polars().is_null().into(); + let expr: NuExpression = expr.into_polars().is_null().into(); expr.to_pipeline_data(plugin, engine, call.head) } _ => Err(cant_convert_err( diff --git a/crates/nu_plugin_polars/src/dataframe/series/n_unique.rs b/crates/nu_plugin_polars/src/dataframe/series/n_unique.rs index be6320208d..5426ef6d1d 100644 --- a/crates/nu_plugin_polars/src/dataframe/series/n_unique.rs +++ b/crates/nu_plugin_polars/src/dataframe/series/n_unique.rs @@ -78,7 +78,7 @@ impl PluginCommand for NUnique { command(plugin, engine, call, lazy.collect(call.head)?) } PolarsPluginObject::NuExpression(expr) => { - let expr: NuExpression = expr.to_polars().n_unique().into(); + let expr: NuExpression = expr.into_polars().n_unique().into(); expr.to_pipeline_data(plugin, engine, call.head) } _ => Err(cant_convert_err( diff --git a/crates/nu_plugin_polars/src/dataframe/series/shift.rs b/crates/nu_plugin_polars/src/dataframe/series/shift.rs index 02e47b55c0..9b5f8b2f63 100644 --- a/crates/nu_plugin_polars/src/dataframe/series/shift.rs +++ b/crates/nu_plugin_polars/src/dataframe/series/shift.rs @@ -112,7 +112,7 @@ fn command_lazy( let lazy: NuLazyFrame = match fill { Some(ref fill) => { - let expr = NuExpression::try_from_value(plugin, fill)?.to_polars(); + let expr = NuExpression::try_from_value(plugin, fill)?.into_polars(); lazy.shift_and_fill(lit(shift), expr).into() } None => lazy.shift(shift).into(), diff --git a/crates/nu_plugin_polars/src/dataframe/values/nu_expression/mod.rs b/crates/nu_plugin_polars/src/dataframe/values/nu_expression/mod.rs index 34f655a608..96eab00d53 100644 --- a/crates/nu_plugin_polars/src/dataframe/values/nu_expression/mod.rs +++ b/crates/nu_plugin_polars/src/dataframe/values/nu_expression/mod.rs @@ -71,7 +71,7 @@ impl NuExpression { } } - pub fn to_polars(self) -> Expr { + pub fn into_polars(self) -> Expr { self.expr.expect("Expression cannot be none to convert") } @@ -121,7 +121,7 @@ impl ExtractedExpr { match value { Value::String { val, .. } => Ok(ExtractedExpr::Single(col(val.as_str()))), Value::Custom { .. } => NuExpression::try_from_value(plugin, &value) - .map(NuExpression::to_polars) + .map(NuExpression::into_polars) .map(ExtractedExpr::Single), Value::List { vals, .. } => vals .into_iter() diff --git a/crates/nu_plugin_polars/src/dataframe/values/nu_lazyframe/mod.rs b/crates/nu_plugin_polars/src/dataframe/values/nu_lazyframe/mod.rs index 86267dd51b..0f8d80c4b9 100644 --- a/crates/nu_plugin_polars/src/dataframe/values/nu_lazyframe/mod.rs +++ b/crates/nu_plugin_polars/src/dataframe/values/nu_lazyframe/mod.rs @@ -72,7 +72,7 @@ impl NuLazyFrame { F: Fn(LazyFrame, Expr) -> LazyFrame, { let df = self.to_polars(); - let expr = expr.to_polars(); + let expr = expr.into_polars(); let new_frame = f(df, expr); Self::new(self.from_eager, new_frame) } From ad6deadf247164016318ed8002fd9b310fe8f8b8 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Thu, 2 May 2024 16:51:16 -0700 Subject: [PATCH 07/16] Flush on every plugin `Data` message (#12728) # Description This helps to ensure data produced on a stream is immediately available to the consumer of the stream. The BufWriter introduced for performance reasons in 0.93 exposed the behavior that data messages wouldn't make it to the other side until they filled the buffer in @cablehead's [`nu_plugin_from_sse`](https://github.com/cablehead/nu_plugin_from_sse). I had originally not flushed on every `Data` message because I figured that it isn't really critical that the other side sees those messages immediately, since they're not used for control and they are flushed when waiting for acknowledgement or when the buffer is too full anyway. Increasing the amount of data that can be sent with a single underlying write increases performance, but this interferes with some plugins that want to use streams in a more real-time way. In the future I would like to make this configurable, maybe even per-command, so that a command can decide what the priority is. But for now I think this is reasonable. In the worst case, this decreases performance by about 40%, when sending very small values (just numbers). But for larger values, this PR actually increases performance by about 20%, because I've increased the buffer size about 2x to 16,384 bytes. The previous value of 8,192 bytes was too small to fit a full buffer coming from an external command, so doubling it makes sense, and now a write of a buffer from an external command can be done in exactly one write call, which I think makes sense. I'm doing this at the same time because flushing each data message would make it very likely that each individual data message from an external stream would require exactly two writes rather than approximately one (amortized). Again, hopefully the tradeoff isn't too bad, and if it is I'll just make it configurable. # User-Facing Changes - Performance of plugin streams will be a bit different - Plugins that expect to send streams in real-time will work again # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` --- crates/nu-plugin-core/src/interface/stream/mod.rs | 7 +++++-- crates/nu-plugin-engine/src/init.rs | 5 ++++- crates/nu-plugin/src/plugin/mod.rs | 5 ++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/crates/nu-plugin-core/src/interface/stream/mod.rs b/crates/nu-plugin-core/src/interface/stream/mod.rs index e8f48939dd..541e1176a0 100644 --- a/crates/nu-plugin-core/src/interface/stream/mod.rs +++ b/crates/nu-plugin-core/src/interface/stream/mod.rs @@ -202,10 +202,13 @@ where if !self.ended { self.writer .write_stream_message(StreamMessage::Data(self.id, data.into()))?; + // Flush after each data message to ensure they do predictably appear on the other side + // when they're generated + // + // TODO: make the buffering configurable, as this is a factor for performance + self.writer.flush()?; // This implements flow control, so we don't write too many messages: if !self.signal.notify_sent()? { - // Flush the output, and then wait for acknowledgements - self.writer.flush()?; self.signal.wait_for_drain() } else { Ok(()) diff --git a/crates/nu-plugin-engine/src/init.rs b/crates/nu-plugin-engine/src/init.rs index 2092935fad..0ba70b49c0 100644 --- a/crates/nu-plugin-engine/src/init.rs +++ b/crates/nu-plugin-engine/src/init.rs @@ -24,7 +24,10 @@ use crate::{ PluginSource, }; -pub(crate) const OUTPUT_BUFFER_SIZE: usize = 8192; +/// This should be larger than the largest commonly sent message to avoid excessive fragmentation. +/// +/// The buffers coming from external streams are typically each 8192 bytes, so double that. +pub(crate) const OUTPUT_BUFFER_SIZE: usize = 16384; /// Spawn the command for a plugin, in the given `mode`. After spawning, it can be passed to /// [`make_plugin_interface()`] to get a [`PluginInterface`]. diff --git a/crates/nu-plugin/src/plugin/mod.rs b/crates/nu-plugin/src/plugin/mod.rs index c7283d10f3..0ec170f4cd 100644 --- a/crates/nu-plugin/src/plugin/mod.rs +++ b/crates/nu-plugin/src/plugin/mod.rs @@ -28,8 +28,11 @@ mod interface; pub use command::{create_plugin_signature, PluginCommand, SimplePluginCommand}; pub use interface::{EngineInterface, EngineInterfaceManager}; +/// This should be larger than the largest commonly sent message to avoid excessive fragmentation. +/// +/// The buffers coming from external streams are typically each 8192 bytes, so double that. #[allow(dead_code)] -pub(crate) const OUTPUT_BUFFER_SIZE: usize = 8192; +pub(crate) const OUTPUT_BUFFER_SIZE: usize = 16384; /// The API for a Nushell plugin /// From 847646e44e4fbd70e574ca389935cf8876865d60 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Fri, 3 May 2024 00:36:10 +0000 Subject: [PATCH 08/16] Remove lazy records (#12682) # Description Removes lazy records from the language, following from the reasons outlined in #12622. Namely, this should make semantics more clear and will eliminate concerns regarding maintainability. # User-Facing Changes - Breaking change: `lazy make` is removed. - Breaking change: `describe --collect-lazyrecords` flag is removed. - `sys` and `debug info` now return regular records. # After Submitting - Update nushell book if necessary. - Explore new `sys` and `debug info` APIs to prevent them from taking too long (e.g., subcommands or taking an optional column/cell-path argument). --- .../src/completions/variable_completions.rs | 29 -- .../nu-cmd-lang/src/core_commands/describe.rs | 75 +---- .../src/core_commands/lazy_make.rs | 179 ----------- crates/nu-cmd-lang/src/core_commands/mod.rs | 2 - crates/nu-cmd-lang/src/default_context.rs | 1 - crates/nu-cmd-lang/src/example_support.rs | 4 - crates/nu-color-config/src/style_computer.rs | 7 +- crates/nu-command/src/debug/explain.rs | 4 - crates/nu-command/src/debug/info.rs | 301 ++++++------------ crates/nu-command/src/filters/columns.rs | 12 - crates/nu-command/src/filters/find.rs | 9 - crates/nu-command/src/filters/items.rs | 6 - crates/nu-command/src/filters/values.rs | 14 - crates/nu-command/src/formats/to/json.rs | 4 - crates/nu-command/src/formats/to/msgpack.rs | 3 - crates/nu-command/src/formats/to/text.rs | 4 - crates/nu-command/src/formats/to/toml.rs | 4 - crates/nu-command/src/formats/to/yaml.rs | 4 - crates/nu-command/src/system/sys.rs | 48 +-- crates/nu-command/src/viewers/table.rs | 4 - crates/nu-command/tests/commands/insert.rs | 15 - crates/nu-command/tests/commands/update.rs | 7 - crates/nu-command/tests/commands/upsert.rs | 11 - crates/nu-explore/src/nu_common/value.rs | 4 - .../src/util/with_custom_values_in.rs | 38 +-- .../src/plugin_custom_value/mod.rs | 15 - .../nu-plugin-test-support/src/plugin_test.rs | 3 - crates/nu-plugin/src/plugin/command.rs | 5 - crates/nu-protocol/src/errors/shell_error.rs | 10 - crates/nu-protocol/src/value/lazy_record.rs | 29 -- crates/nu-protocol/src/value/mod.rs | 145 +-------- crates/nuon/src/to.rs | 4 - 32 files changed, 133 insertions(+), 867 deletions(-) delete mode 100644 crates/nu-cmd-lang/src/core_commands/lazy_make.rs delete mode 100644 crates/nu-protocol/src/value/lazy_record.rs diff --git a/crates/nu-cli/src/completions/variable_completions.rs b/crates/nu-cli/src/completions/variable_completions.rs index c8cadc0d0b..b869e9f972 100644 --- a/crates/nu-cli/src/completions/variable_completions.rs +++ b/crates/nu-cli/src/completions/variable_completions.rs @@ -267,24 +267,6 @@ fn nested_suggestions( output } - Value::LazyRecord { val, .. } => { - // Add all the columns as completion - for column_name in val.column_names() { - output.push(SemanticSuggestion { - suggestion: Suggestion { - value: column_name.to_string(), - description: None, - style: None, - extra: None, - span: current_span, - append_whitespace: false, - }, - kind: Some(kind.clone()), - }); - } - - output - } Value::List { vals, .. } => { for column_name in get_columns(vals.as_slice()) { output.push(SemanticSuggestion { @@ -321,17 +303,6 @@ fn recursive_value(val: &Value, sublevels: &[Vec]) -> Result { Err(span) } } - Value::LazyRecord { val, .. } => { - for col in val.column_names() { - if col.as_bytes() == *sublevel { - let val = val.get_column_value(col).map_err(|_| span)?; - return recursive_value(&val, next_sublevels); - } - } - - // Current sublevel value not found - Err(span) - } Value::List { vals, .. } => { for col in get_columns(vals.as_slice()) { if col.as_bytes() == *sublevel { diff --git a/crates/nu-cmd-lang/src/core_commands/describe.rs b/crates/nu-cmd-lang/src/core_commands/describe.rs index 58384d837b..48b52036e1 100644 --- a/crates/nu-cmd-lang/src/core_commands/describe.rs +++ b/crates/nu-cmd-lang/src/core_commands/describe.rs @@ -26,7 +26,6 @@ impl Command for Describe { "show detailed information about the value", Some('d'), ) - .switch("collect-lazyrecords", "collect lazy records", Some('l')) .category(Category::Core) } @@ -44,21 +43,7 @@ impl Command for Describe { let options = Options { no_collect: call.has_flag(engine_state, stack, "no-collect")?, detailed: call.has_flag(engine_state, stack, "detailed")?, - collect_lazyrecords: call.has_flag(engine_state, stack, "collect-lazyrecords")?, }; - if options.collect_lazyrecords { - nu_protocol::report_error_new( - engine_state, - &ShellError::GenericError { - error: "Deprecated flag".into(), - msg: "the `--collect-lazyrecords` flag is deprecated, since lazy records will be removed in 0.94.0" - .into(), - span: Some(call.head), - help: None, - inner: vec![], - }, - ); - } run(Some(engine_state), call, input, options) } @@ -71,7 +56,6 @@ impl Command for Describe { let options = Options { no_collect: call.has_flag_const(working_set, "no-collect")?, detailed: call.has_flag_const(working_set, "detailed")?, - collect_lazyrecords: call.has_flag_const(working_set, "collect-lazyrecords")?, }; run(None, call, input, options) } @@ -89,13 +73,11 @@ impl Command for Describe { "{shell:'true', uwu:true, features: {bugs:false, multiplatform:true, speed: 10}, fib: [1 1 2 3 5 8], on_save: {|x| print $'Saving ($x)'}, first_commit: 2019-05-10, my_duration: (4min + 20sec)} | describe -d", result: Some(Value::test_record(record!( "type" => Value::test_string("record"), - "lazy" => Value::test_bool(false), "columns" => Value::test_record(record!( "shell" => Value::test_string("string"), "uwu" => Value::test_string("bool"), "features" => Value::test_record(record!( "type" => Value::test_string("record"), - "lazy" => Value::test_bool(false), "columns" => Value::test_record(record!( "bugs" => Value::test_string("bool"), "multiplatform" => Value::test_string("bool"), @@ -168,7 +150,6 @@ impl Command for Describe { struct Options { no_collect: bool, detailed: bool, - collect_lazyrecords: bool, } fn run( @@ -243,7 +224,7 @@ fn run( if options.no_collect { Value::string("any", head) } else { - describe_value(input.into_value(head), head, engine_state, options)? + describe_value(input.into_value(head), head, engine_state, ) } }, "metadata" => metadata_to_value(metadata, head), @@ -264,7 +245,7 @@ fn run( if !options.detailed { Value::string(value.get_type().to_string(), head) } else { - describe_value(value, head, engine_state, options)? + describe_value(value, head, engine_state) } } }; @@ -288,9 +269,8 @@ fn describe_value( value: Value, head: nu_protocol::Span, engine_state: Option<&EngineState>, - options: Options, -) -> Result { - Ok(match value { +) -> Value { + match value { Value::Custom { val, .. } => Value::record( record!( "type" => Value::string("custom", head), @@ -320,14 +300,12 @@ fn describe_value( std::mem::take(v), head, engine_state, - options, - )?); + )); } Value::record( record!( "type" => Value::string("record", head), - "lazy" => Value::bool(false, head), "columns" => Value::record(val, head), ), head, @@ -338,11 +316,9 @@ fn describe_value( "type" => Value::string("list", head), "length" => Value::int(vals.len() as i64, head), "values" => Value::list(vals.into_iter().map(|v| - Ok(compact_primitive_description( - describe_value(v, head, engine_state, options)? - )) + compact_primitive_description(describe_value(v, head, engine_state)) ) - .collect::, ShellError>>()?, head), + .collect(), head), ), head, ), @@ -394,42 +370,7 @@ fn describe_value( ), head, ), - Value::LazyRecord { val, .. } => { - let mut record = Record::new(); - - record.push("type", Value::string("record", head)); - record.push("lazy", Value::bool(true, head)); - - if options.collect_lazyrecords { - let collected = val.collect()?; - if let Value::Record { val, .. } = - describe_value(collected, head, engine_state, options)? - { - let mut val = Record::clone(&val); - - for (_k, v) in val.iter_mut() { - *v = compact_primitive_description(describe_value( - std::mem::take(v), - head, - engine_state, - options, - )?); - } - - record.push("length", Value::int(val.len() as i64, head)); - record.push("columns", Value::record(val, head)); - } else { - let cols = val.column_names(); - record.push("length", Value::int(cols.len() as i64, head)); - } - } else { - let cols = val.column_names(); - record.push("length", Value::int(cols.len() as i64, head)); - } - - Value::record(record, head) - } - }) + } } fn metadata_to_value(metadata: Option>, head: nu_protocol::Span) -> Value { diff --git a/crates/nu-cmd-lang/src/core_commands/lazy_make.rs b/crates/nu-cmd-lang/src/core_commands/lazy_make.rs deleted file mode 100644 index 7c90a04b78..0000000000 --- a/crates/nu-cmd-lang/src/core_commands/lazy_make.rs +++ /dev/null @@ -1,179 +0,0 @@ -use nu_engine::{command_prelude::*, eval_block}; -use nu_protocol::{debugger::WithoutDebug, engine::Closure, LazyRecord}; -use std::{ - collections::{hash_map::Entry, HashMap}, - sync::{Arc, Mutex}, -}; - -#[derive(Clone)] -pub struct LazyMake; - -impl Command for LazyMake { - fn name(&self) -> &str { - "lazy make" - } - - fn signature(&self) -> Signature { - Signature::build("lazy make") - .input_output_types(vec![(Type::Nothing, Type::record())]) - .required_named( - "columns", - SyntaxShape::List(Box::new(SyntaxShape::String)), - "Closure that gets called when the LazyRecord needs to list the available column names", - Some('c') - ) - .required_named( - "get-value", - SyntaxShape::Closure(Some(vec![SyntaxShape::String])), - "Closure to call when a value needs to be produced on demand", - Some('g') - ) - .category(Category::Core) - } - - fn usage(&self) -> &str { - "Create a lazy record." - } - - fn extra_usage(&self) -> &str { - "Lazy records are special records that only evaluate their values once the property is requested. - For example, when printing a lazy record, all of its fields will be collected. But when accessing - a specific property, only it will be evaluated. - - Note that this is unrelated to the lazyframes feature bundled with dataframes." - } - - fn search_terms(&self) -> Vec<&str> { - vec!["deferred", "record", "procedural"] - } - - fn run( - &self, - engine_state: &EngineState, - stack: &mut Stack, - call: &Call, - _input: PipelineData, - ) -> Result { - nu_protocol::report_error_new( - engine_state, - &ShellError::GenericError { - error: "Deprecated command".into(), - msg: "warning: lazy records and the `lazy make` command will be removed in 0.94.0" - .into(), - span: Some(call.head), - help: None, - inner: vec![], - }, - ); - - let span = call.head; - let columns: Vec> = call - .get_flag(engine_state, stack, "columns")? - .expect("required flag"); - - let get_value: Closure = call - .get_flag(engine_state, stack, "get-value")? - .expect("required flag"); - - let mut unique = HashMap::with_capacity(columns.len()); - - for col in &columns { - match unique.entry(&col.item) { - Entry::Occupied(entry) => { - return Err(ShellError::ColumnDefinedTwice { - col_name: col.item.clone(), - second_use: col.span, - first_use: *entry.get(), - }); - } - Entry::Vacant(entry) => { - entry.insert(col.span); - } - } - } - - let stack = stack.clone().reset_out_dest().capture(); - - Ok(Value::lazy_record( - Box::new(NuLazyRecord { - engine_state: engine_state.clone(), - stack: Arc::new(Mutex::new(stack)), - columns: columns.into_iter().map(|s| s.item).collect(), - get_value, - span, - }), - span, - ) - .into_pipeline_data()) - } - - fn examples(&self) -> Vec { - vec![ - // TODO: Figure out how to "test" these examples, or leave results as None - Example { - description: "Create a lazy record", - example: r#"lazy make --columns ["haskell", "futures", "nushell"] --get-value { |lazything| $lazything + "!" }"#, - result: None, - }, - Example { - description: "Test the laziness of lazy records", - example: r#"lazy make --columns ["hello"] --get-value { |key| print $"getting ($key)!"; $key | str upcase }"#, - result: None, - }, - ] - } -} - -#[derive(Clone)] -struct NuLazyRecord { - engine_state: EngineState, - stack: Arc>, - columns: Vec, - get_value: Closure, - span: Span, -} - -impl std::fmt::Debug for NuLazyRecord { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("NuLazyRecord").finish() - } -} - -impl<'a> LazyRecord<'a> for NuLazyRecord { - fn column_names(&'a self) -> Vec<&'a str> { - self.columns.iter().map(|column| column.as_str()).collect() - } - - fn get_column_value(&self, column: &str) -> Result { - let block = self.engine_state.get_block(self.get_value.block_id); - let mut stack = self.stack.lock().expect("lock must not be poisoned"); - let column_value = Value::string(column, self.span); - - if let Some(var) = block.signature.get_positional(0) { - if let Some(var_id) = &var.var_id { - stack.add_var(*var_id, column_value.clone()); - } - } - - let pipeline_result = eval_block::( - &self.engine_state, - &mut stack, - block, - PipelineData::Value(column_value, None), - ); - - pipeline_result.map(|data| match data { - PipelineData::Value(value, ..) => value, - // TODO: Proper error handling. - _ => Value::nothing(self.span), - }) - } - - fn span(&self) -> Span { - self.span - } - - fn clone_value(&self, span: Span) -> Value { - Value::lazy_record(Box::new((*self).clone()), span) - } -} diff --git a/crates/nu-cmd-lang/src/core_commands/mod.rs b/crates/nu-cmd-lang/src/core_commands/mod.rs index 87e42783e1..2b865641e8 100644 --- a/crates/nu-cmd-lang/src/core_commands/mod.rs +++ b/crates/nu-cmd-lang/src/core_commands/mod.rs @@ -21,7 +21,6 @@ mod hide; mod hide_env; mod if_; mod ignore; -mod lazy_make; mod let_; mod loop_; mod match_; @@ -58,7 +57,6 @@ pub use hide::Hide; pub use hide_env::HideEnv; pub use if_::If; pub use ignore::Ignore; -pub use lazy_make::LazyMake; pub use let_::Let; pub use loop_::Loop; pub use match_::Match; diff --git a/crates/nu-cmd-lang/src/default_context.rs b/crates/nu-cmd-lang/src/default_context.rs index 2e1681af74..43fa0ddbf5 100644 --- a/crates/nu-cmd-lang/src/default_context.rs +++ b/crates/nu-cmd-lang/src/default_context.rs @@ -43,7 +43,6 @@ pub fn create_default_context() -> EngineState { OverlayList, OverlayNew, OverlayHide, - LazyMake, Let, Loop, Match, diff --git a/crates/nu-cmd-lang/src/example_support.rs b/crates/nu-cmd-lang/src/example_support.rs index 5b9b0b3a07..42dcd447b7 100644 --- a/crates/nu-cmd-lang/src/example_support.rs +++ b/crates/nu-cmd-lang/src/example_support.rs @@ -306,10 +306,6 @@ impl<'a> std::fmt::Debug for DebuggableValue<'a> { Value::Custom { val, .. } => { write!(f, "CustomValue({:?})", val) } - Value::LazyRecord { val, .. } => { - let rec = val.collect().map_err(|_| std::fmt::Error)?; - write!(f, "LazyRecord({:?})", DebuggableValue(&rec)) - } } } } diff --git a/crates/nu-color-config/src/style_computer.rs b/crates/nu-color-config/src/style_computer.rs index 3d21e1583c..cd2454f011 100644 --- a/crates/nu-color-config/src/style_computer.rs +++ b/crates/nu-color-config/src/style_computer.rs @@ -106,10 +106,9 @@ impl<'a> StyleComputer<'a> { Value::Binary { .. } => TextStyle::with_style(Left, s), Value::CellPath { .. } => TextStyle::with_style(Left, s), Value::Record { .. } | Value::List { .. } => TextStyle::with_style(Left, s), - Value::Closure { .. } - | Value::Custom { .. } - | Value::Error { .. } - | Value::LazyRecord { .. } => TextStyle::basic_left(), + Value::Closure { .. } | Value::Custom { .. } | Value::Error { .. } => { + TextStyle::basic_left() + } } } diff --git a/crates/nu-command/src/debug/explain.rs b/crates/nu-command/src/debug/explain.rs index bdfb61bb48..0abd87b9a3 100644 --- a/crates/nu-command/src/debug/explain.rs +++ b/crates/nu-command/src/debug/explain.rs @@ -272,10 +272,6 @@ pub fn debug_string_without_formatting(value: &Value) -> String { .collect::>() .join(" ") ), - Value::LazyRecord { val, .. } => match val.collect() { - Ok(val) => debug_string_without_formatting(&val), - Err(error) => format!("{error:?}"), - }, //TODO: It would be good to drill deeper into closures. Value::Closure { val, .. } => format!("", val.block_id), Value::Nothing { .. } => String::new(), diff --git a/crates/nu-command/src/debug/info.rs b/crates/nu-command/src/debug/info.rs index 711fd58c16..76317ed866 100644 --- a/crates/nu-command/src/debug/info.rs +++ b/crates/nu-command/src/debug/info.rs @@ -1,5 +1,4 @@ use nu_engine::command_prelude::*; -use nu_protocol::LazyRecord; use sysinfo::{MemoryRefreshKind, Pid, ProcessRefreshKind, RefreshKind, System}; const ENV_PATH_SEPARATOR_CHAR: char = { @@ -39,14 +38,10 @@ impl Command for DebugInfo { &self, _engine_state: &EngineState, _stack: &mut Stack, - _call: &Call, + call: &Call, _input: PipelineData, ) -> Result { - let span = Span::unknown(); - - let record = LazySystemInfoRecord { span }; - - Ok(Value::lazy_record(Box::new(record), span).into_pipeline_data()) + Ok(all_columns(call.head).into_pipeline_data()) } fn examples(&self) -> Vec { @@ -58,207 +53,119 @@ impl Command for DebugInfo { } } -#[derive(Debug, Clone)] -struct LazySystemInfoRecord { - span: Span, -} +fn all_columns(span: Span) -> Value { + let rk = RefreshKind::new() + .with_processes(ProcessRefreshKind::everything()) + .with_memory(MemoryRefreshKind::everything()); -impl LazySystemInfoRecord { - fn get_column_value_with_system( - &self, - column: &str, - system_option: Option<&System>, - ) -> Result { - let pid = Pid::from(std::process::id() as usize); - match column { - "thread_id" => Ok(Value::int(get_thread_id() as i64, self.span)), - "pid" => Ok(Value::int(pid.as_u32() as i64, self.span)), - "ppid" => { - // only get information requested - let system_opt = SystemOpt::from((system_option, || { - RefreshKind::new().with_processes(ProcessRefreshKind::everything()) - })); + // only get information requested + let sys = System::new_with_specifics(rk); - let system = system_opt.get_system(); - // get the process information for the nushell pid - let pinfo = system.process(pid); + let pid = Pid::from(std::process::id() as usize); + let ppid = { + sys.process(pid) + .and_then(|p| p.parent()) + .map(|p| Value::int(p.as_u32().into(), span)) + .unwrap_or(Value::nothing(span)) + }; - Ok(pinfo - .and_then(|p| p.parent()) - .map(|p| Value::int(p.as_u32() as i64, self.span)) - .unwrap_or(Value::nothing(self.span))) - } - "system" => { - // only get information requested - let system_opt = SystemOpt::from((system_option, || { - RefreshKind::new().with_memory(MemoryRefreshKind::everything()) - })); + let system = Value::record( + record! { + "total_memory" => Value::filesize(sys.total_memory() as i64, span), + "free_memory" => Value::filesize(sys.free_memory() as i64, span), + "used_memory" => Value::filesize(sys.used_memory() as i64, span), + "available_memory" => Value::filesize(sys.available_memory() as i64, span), + }, + span, + ); - let system = system_opt.get_system(); + let process = if let Some(p) = sys.process(pid) { + let root = if let Some(path) = p.exe().and_then(|p| p.parent()) { + Value::string(path.to_string_lossy().to_string(), span) + } else { + Value::nothing(span) + }; - Ok(Value::record( - record! { - "total_memory" => Value::filesize(system.total_memory() as i64, self.span), - "free_memory" => Value::filesize(system.free_memory() as i64, self.span), - "used_memory" => Value::filesize(system.used_memory() as i64, self.span), - "available_memory" => Value::filesize(system.available_memory() as i64, self.span), - }, - self.span, - )) - } - "process" => { - // only get information requested - let system_opt = SystemOpt::from((system_option, || { - RefreshKind::new().with_processes(ProcessRefreshKind::everything()) - })); + let cwd = if let Some(path) = p.cwd() { + Value::string(path.to_string_lossy().to_string(), span) + } else { + Value::nothing(span) + }; - let system = system_opt.get_system(); - let pinfo = system.process(pid); + let exe_path = if let Some(path) = p.exe() { + Value::string(path.to_string_lossy().to_string(), span) + } else { + Value::nothing(span) + }; - if let Some(p) = pinfo { - Ok(Value::record( - record! { - "memory" => Value::filesize(p.memory() as i64, self.span), - "virtual_memory" => Value::filesize(p.virtual_memory() as i64, self.span), - "status" => Value::string(p.status().to_string(), self.span), - "root" => { - if let Some(path) = p.exe().and_then(|p| p.parent()) { - Value::string(path.to_string_lossy().to_string(), self.span) - } else { - Value::nothing(self.span) - } - }, - "cwd" => { - if let Some(path) = p.cwd() { - Value::string(path.to_string_lossy().to_string(), self.span) - }else{ - Value::nothing(self.span) - } - }, - "exe_path" => { - if let Some(path)= p.exe() { - Value::string(path.to_string_lossy().to_string(), self.span) - }else{ - Value::nothing(self.span) - } - }, - "command" => Value::string(p.cmd().join(" "), self.span), - "name" => Value::string(p.name().to_string(), self.span), - "environment" => { - let mut env_rec = Record::new(); - for val in p.environ() { - if let Some((key, value)) = val.split_once('=') { - let is_env_var_a_list = { - { - #[cfg(target_family = "windows")] - { - key == "Path" || key == "PATHEXT" || key == "PSMODULEPATH" || key == "PSModulePath" - } - #[cfg(not(target_family = "windows"))] - { - key == "PATH" || key == "DYLD_FALLBACK_LIBRARY_PATH" - } - } - }; - if is_env_var_a_list { - let items = value.split(ENV_PATH_SEPARATOR_CHAR).map(|r| Value::string(r.to_string(), self.span)).collect::>(); - env_rec.push(key.to_string(), Value::list(items, self.span)); - } else if key == "LS_COLORS" { // LS_COLORS is a special case, it's a colon separated list of key=value pairs - let items = value.split(':').map(|r| Value::string(r.to_string(), self.span)).collect::>(); - env_rec.push(key.to_string(), Value::list(items, self.span)); - } else { - env_rec.push(key.to_string(), Value::string(value.to_string(), self.span)); - } - } - } - Value::record(env_rec, self.span) - }, - }, - self.span, - )) - } else { - // If we can't get the process information, just return the system information - // only get information requested - let system_opt = SystemOpt::from((system_option, || { - RefreshKind::new().with_memory(MemoryRefreshKind::everything()) - })); - let system = system_opt.get_system(); - - Ok(Value::record( - record! { - "total_memory" => Value::filesize(system.total_memory() as i64, self.span), - "free_memory" => Value::filesize(system.free_memory() as i64, self.span), - "used_memory" => Value::filesize(system.used_memory() as i64, self.span), - "available_memory" => Value::filesize(system.available_memory() as i64, self.span), - }, - self.span, - )) + let environment = { + let mut env_rec = Record::new(); + for val in p.environ() { + if let Some((key, value)) = val.split_once('=') { + let is_env_var_a_list = { + { + #[cfg(target_family = "windows")] + { + key == "Path" + || key == "PATHEXT" + || key == "PSMODULEPATH" + || key == "PSModulePath" + } + #[cfg(not(target_family = "windows"))] + { + key == "PATH" || key == "DYLD_FALLBACK_LIBRARY_PATH" + } + } + }; + if is_env_var_a_list { + let items = value + .split(ENV_PATH_SEPARATOR_CHAR) + .map(|r| Value::string(r.to_string(), span)) + .collect::>(); + env_rec.push(key.to_string(), Value::list(items, span)); + } else if key == "LS_COLORS" { + // LS_COLORS is a special case, it's a colon separated list of key=value pairs + let items = value + .split(':') + .map(|r| Value::string(r.to_string(), span)) + .collect::>(); + env_rec.push(key.to_string(), Value::list(items, span)); + } else { + env_rec.push(key.to_string(), Value::string(value.to_string(), span)); + } } } - _ => Err(ShellError::IncompatibleParametersSingle { - msg: format!("Unknown column: {}", column), - span: self.span, - }), - } - } -} + Value::record(env_rec, span) + }; -impl<'a> LazyRecord<'a> for LazySystemInfoRecord { - fn column_names(&'a self) -> Vec<&'a str> { - vec!["thread_id", "pid", "ppid", "process", "system"] - } + Value::record( + record! { + "memory" => Value::filesize(p.memory() as i64, span), + "virtual_memory" => Value::filesize(p.virtual_memory() as i64, span), + "status" => Value::string(p.status().to_string(), span), + "root" => root, + "cwd" => cwd, + "exe_path" => exe_path, + "command" => Value::string(p.cmd().join(" "), span), + "name" => Value::string(p.name(), span), + "environment" => environment, + }, + span, + ) + } else { + Value::nothing(span) + }; - fn get_column_value(&self, column: &str) -> Result { - self.get_column_value_with_system(column, None) - } - - fn span(&self) -> Span { - self.span - } - - fn clone_value(&self, span: Span) -> Value { - Value::lazy_record(Box::new(LazySystemInfoRecord { span }), span) - } - - fn collect(&'a self) -> Result { - let rk = RefreshKind::new() - .with_processes(ProcessRefreshKind::everything()) - .with_memory(MemoryRefreshKind::everything()); - // only get information requested - let system = System::new_with_specifics(rk); - - self.column_names() - .into_iter() - .map(|col| { - let val = self.get_column_value_with_system(col, Some(&system))?; - Ok((col.to_owned(), val)) - }) - .collect::>() - .map(|record| Value::record(record, self.span())) - } -} - -enum SystemOpt<'a> { - Ptr(&'a System), - Owned(Box), -} - -impl<'a> SystemOpt<'a> { - fn get_system(&'a self) -> &'a System { - match self { - SystemOpt::Ptr(system) => system, - SystemOpt::Owned(system) => system, - } - } -} - -impl<'a, F: Fn() -> RefreshKind> From<(Option<&'a System>, F)> for SystemOpt<'a> { - fn from((system_opt, refresh_kind_create): (Option<&'a System>, F)) -> Self { - match system_opt { - Some(system) => SystemOpt::<'a>::Ptr(system), - None => SystemOpt::Owned(Box::new(System::new_with_specifics(refresh_kind_create()))), - } - } + Value::record( + record! { + "thread_id" => Value::int(get_thread_id() as i64, span), + "pid" => Value::int(pid.as_u32().into(), span), + "ppid" => ppid, + "system" => system, + "process" => process, + }, + span, + ) } fn get_thread_id() -> u64 { diff --git a/crates/nu-command/src/filters/columns.rs b/crates/nu-command/src/filters/columns.rs index a5b0f2b9ea..44b713e793 100644 --- a/crates/nu-command/src/filters/columns.rs +++ b/crates/nu-command/src/filters/columns.rs @@ -105,18 +105,6 @@ fn getcol( .into_pipeline_data(ctrlc) .set_metadata(metadata)) } - Value::LazyRecord { val, .. } => { - Ok({ - // Unfortunate casualty to LazyRecord's column_names not generating 'static strs - let cols: Vec<_> = - val.column_names().iter().map(|s| s.to_string()).collect(); - - cols.into_iter() - .map(move |x| Value::string(x, head)) - .into_pipeline_data(ctrlc) - .set_metadata(metadata) - }) - } Value::Record { val, .. } => Ok(val .into_owned() .into_iter() diff --git a/crates/nu-command/src/filters/find.rs b/crates/nu-command/src/filters/find.rs index 9cc140ece1..6efda0d836 100644 --- a/crates/nu-command/src/filters/find.rs +++ b/crates/nu-command/src/filters/find.rs @@ -533,15 +533,6 @@ fn value_should_be_printed( Value::Record { val, .. } => { record_matches_term(val, columns_to_search, filter_config, term, span) } - Value::LazyRecord { val, .. } => match val.collect() { - Ok(val) => match val { - Value::Record { val, .. } => { - record_matches_term(&val, columns_to_search, filter_config, term, span) - } - _ => false, - }, - Err(_) => false, - }, Value::Binary { .. } => false, }); if invert { diff --git a/crates/nu-command/src/filters/items.rs b/crates/nu-command/src/filters/items.rs index 8dca421200..5a97a5a3db 100644 --- a/crates/nu-command/src/filters/items.rs +++ b/crates/nu-command/src/filters/items.rs @@ -44,12 +44,6 @@ impl Command for Items { match input { PipelineData::Empty => Ok(PipelineData::Empty), PipelineData::Value(value, ..) => { - let value = if let Value::LazyRecord { val, .. } = value { - val.collect()? - } else { - value - }; - let span = value.span(); match value { Value::Record { val, .. } => { diff --git a/crates/nu-command/src/filters/values.rs b/crates/nu-command/src/filters/values.rs index 344ffc96c0..8b35352150 100644 --- a/crates/nu-command/src/filters/values.rs +++ b/crates/nu-command/src/filters/values.rs @@ -161,20 +161,6 @@ fn values( .cloned() .collect::>() .into_pipeline_data_with_metadata(metadata, ctrlc)), - Value::LazyRecord { val, .. } => { - let record = match val.collect()? { - Value::Record { val, .. } => val, - _ => Err(ShellError::NushellFailedSpanned { - msg: "`LazyRecord::collect()` promises `Value::Record`".into(), - label: "Violating lazy record found here".into(), - span, - })?, - }; - Ok(record - .into_owned() - .into_values() - .into_pipeline_data_with_metadata(metadata, ctrlc)) - } // Propagate errors Value::Error { error, .. } => Err(*error), other => Err(ShellError::OnlySupportsThisInputType { diff --git a/crates/nu-command/src/formats/to/json.rs b/crates/nu-command/src/formats/to/json.rs index c48daad085..0796abc8dc 100644 --- a/crates/nu-command/src/formats/to/json.rs +++ b/crates/nu-command/src/formats/to/json.rs @@ -135,10 +135,6 @@ pub fn value_to_json_value(v: &Value) -> Result { } nu_json::Value::Object(m) } - Value::LazyRecord { val, .. } => { - let collected = val.collect()?; - value_to_json_value(&collected)? - } Value::Custom { val, .. } => { let collected = val.to_base_value(span)?; value_to_json_value(&collected)? diff --git a/crates/nu-command/src/formats/to/msgpack.rs b/crates/nu-command/src/formats/to/msgpack.rs index 9e484eb1bd..a4575f37e4 100644 --- a/crates/nu-command/src/formats/to/msgpack.rs +++ b/crates/nu-command/src/formats/to/msgpack.rs @@ -246,9 +246,6 @@ pub(crate) fn write_value( Value::Custom { val, .. } => { write_value(out, &val.to_base_value(span)?, depth)?; } - Value::LazyRecord { val, .. } => { - write_value(out, &val.collect()?, depth)?; - } } Ok(()) } diff --git a/crates/nu-command/src/formats/to/text.rs b/crates/nu-command/src/formats/to/text.rs index 362786f1c0..2515ca5f92 100644 --- a/crates/nu-command/src/formats/to/text.rs +++ b/crates/nu-command/src/formats/to/text.rs @@ -130,10 +130,6 @@ fn local_into_string(value: Value, separator: &str, config: &Config) -> String { .map(|(x, y)| format!("{}: {}", x, local_into_string(y, ", ", config))) .collect::>() .join(separator), - Value::LazyRecord { val, .. } => match val.collect() { - Ok(val) => local_into_string(val, separator, config), - Err(error) => format!("{error:?}"), - }, Value::Closure { val, .. } => format!("", val.block_id), Value::Nothing { .. } => String::new(), Value::Error { error, .. } => format!("{error:?}"), diff --git a/crates/nu-command/src/formats/to/toml.rs b/crates/nu-command/src/formats/to/toml.rs index adfdf7f39a..385e9576f3 100644 --- a/crates/nu-command/src/formats/to/toml.rs +++ b/crates/nu-command/src/formats/to/toml.rs @@ -62,10 +62,6 @@ fn helper(engine_state: &EngineState, v: &Value) -> Result { - let collected = val.collect()?; - helper(engine_state, &collected)? - } Value::List { vals, .. } => toml::Value::Array(toml_list(engine_state, vals)?), Value::Closure { .. } => { let code = engine_state.get_span_contents(span); diff --git a/crates/nu-command/src/formats/to/yaml.rs b/crates/nu-command/src/formats/to/yaml.rs index d8cdaac725..d03c886328 100644 --- a/crates/nu-command/src/formats/to/yaml.rs +++ b/crates/nu-command/src/formats/to/yaml.rs @@ -62,10 +62,6 @@ pub fn value_to_yaml_value(v: &Value) -> Result { } serde_yaml::Value::Mapping(m) } - Value::LazyRecord { val, .. } => { - let collected = val.collect()?; - value_to_yaml_value(&collected)? - } Value::List { vals, .. } => { let mut out = vec![]; diff --git a/crates/nu-command/src/system/sys.rs b/crates/nu-command/src/system/sys.rs index 1fe41ac7c2..0a836f894a 100644 --- a/crates/nu-command/src/system/sys.rs +++ b/crates/nu-command/src/system/sys.rs @@ -1,6 +1,5 @@ use chrono::{DateTime, Local}; use nu_engine::command_prelude::*; -use nu_protocol::LazyRecord; use std::time::{Duration, UNIX_EPOCH}; use sysinfo::{ Components, CpuRefreshKind, Disks, Networks, System, Users, MINIMUM_CPU_UPDATE_INTERVAL, @@ -32,10 +31,7 @@ impl Command for Sys { call: &Call, _input: PipelineData, ) -> Result { - let span = call.span(); - let ret = Value::lazy_record(Box::new(SysResult { span }), span); - - Ok(ret.into_pipeline_data()) + Ok(all_columns(call.head).into_pipeline_data()) } fn examples(&self) -> Vec { @@ -64,36 +60,18 @@ pub struct SysResult { pub span: Span, } -impl LazyRecord<'_> for SysResult { - fn column_names(&self) -> Vec<&'static str> { - vec!["host", "cpu", "disks", "mem", "temp", "net"] - } - - fn get_column_value(&self, column: &str) -> Result { - let span = self.span; - - match column { - "host" => Ok(host(span)), - "cpu" => Ok(cpu(span)), - "disks" => Ok(disks(span)), - "mem" => Ok(mem(span)), - "temp" => Ok(temp(span)), - "net" => Ok(net(span)), - _ => Err(ShellError::LazyRecordAccessFailed { - message: format!("Could not find column '{column}'"), - column_name: column.to_string(), - span, - }), - } - } - - fn span(&self) -> Span { - self.span - } - - fn clone_value(&self, span: Span) -> Value { - Value::lazy_record(Box::new((*self).clone()), span) - } +fn all_columns(span: Span) -> Value { + Value::record( + record! { + "host" => host(span), + "cpu" => cpu(span), + "disks" => disks(span), + "mem" => mem(span), + "temp" => temp(span), + "net" => net(span), + }, + span, + ) } pub fn trim_cstyle_null(s: String) -> String { diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs index 11731e61b9..e7d2774a5e 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -394,10 +394,6 @@ fn handle_table_command( input.data = PipelineData::Empty; handle_record(input, cfg, val.into_owned()) } - PipelineData::Value(Value::LazyRecord { val, .. }, ..) => { - input.data = val.collect()?.into_pipeline_data(); - handle_table_command(input, cfg) - } PipelineData::Value(Value::Error { error, .. }, ..) => { // Propagate this error outward, so that it goes to stderr // instead of stdout. diff --git a/crates/nu-command/tests/commands/insert.rs b/crates/nu-command/tests/commands/insert.rs index 400319c4f0..c77fb6c867 100644 --- a/crates/nu-command/tests/commands/insert.rs +++ b/crates/nu-command/tests/commands/insert.rs @@ -98,21 +98,6 @@ fn insert_uses_enumerate_index() { assert_eq!(actual.out, "[[index, a, b]; [0, 7, 8], [1, 6, 8]]"); } -#[test] -fn insert_support_lazy_record() { - let actual = - nu!(r#"let x = (lazy make -c ["h"] -g {|a| $a | str upcase}); $x | insert a 10 | get a"#); - assert_eq!(actual.out, "10"); -} - -#[test] -fn lazy_record_test_values() { - let actual = nu!( - r#"lazy make --columns ["haskell", "futures", "nushell"] --get-value { |lazything| $lazything + "!" } | values | length"# - ); - assert_eq!(actual.out, "3"); -} - #[test] fn deep_cell_path_creates_all_nested_records() { let actual = nu!("{a: {}} | insert a.b.c 0 | get a.b.c"); diff --git a/crates/nu-command/tests/commands/update.rs b/crates/nu-command/tests/commands/update.rs index 2e74657f7c..876ed471e5 100644 --- a/crates/nu-command/tests/commands/update.rs +++ b/crates/nu-command/tests/commands/update.rs @@ -103,13 +103,6 @@ fn update_uses_enumerate_index() { assert_eq!(actual.out, "[[index, a]; [0, 8], [1, 8]]"); } -#[test] -fn update_support_lazy_record() { - let actual = - nu!(r#"let x = (lazy make -c ["h"] -g {|a| $a | str upcase}); $x | update h 10 | get h"#); - assert_eq!(actual.out, "10"); -} - #[test] fn list_replacement_closure() { let actual = nu!("[1, 2] | update 1 {|i| $i + 1 } | to nuon"); diff --git a/crates/nu-command/tests/commands/upsert.rs b/crates/nu-command/tests/commands/upsert.rs index 1bb823e88c..0ce8307e70 100644 --- a/crates/nu-command/tests/commands/upsert.rs +++ b/crates/nu-command/tests/commands/upsert.rs @@ -112,17 +112,6 @@ fn upsert_past_end_of_list_stream() { .contains("can't insert at index (the next available index is 3)")); } -#[test] -fn upsert_support_lazy_record() { - let actual = - nu!(r#"let x = (lazy make -c ["h"] -g {|a| $a | str upcase}); $x | upsert h 10 | get h"#); - assert_eq!(actual.out, "10"); - - let actual = - nu!(r#"let x = (lazy make -c ["h"] -g {|a| $a | str upcase}); $x | upsert aa 10 | get aa"#); - assert_eq!(actual.out, "10"); -} - #[test] fn deep_cell_path_creates_all_nested_records() { let actual = nu!("{a: {}} | upsert a.b.c 0 | get a.b.c"); diff --git a/crates/nu-explore/src/nu_common/value.rs b/crates/nu-explore/src/nu_common/value.rs index 2c2c858db0..c61533770b 100644 --- a/crates/nu-explore/src/nu_common/value.rs +++ b/crates/nu-explore/src/nu_common/value.rs @@ -112,10 +112,6 @@ pub fn collect_input(value: Value) -> Result<(Vec, Vec>)> { Ok((vec![String::from("")], lines)) } - Value::LazyRecord { val, .. } => { - let materialized = val.collect()?; - collect_input(materialized) - } Value::Nothing { .. } => Ok((vec![], vec![])), Value::Custom { val, .. } => { let materialized = val.to_base_value(span)?; diff --git a/crates/nu-plugin-core/src/util/with_custom_values_in.rs b/crates/nu-plugin-core/src/util/with_custom_values_in.rs index f9b12223ab..8fcd843ac4 100644 --- a/crates/nu-plugin-core/src/util/with_custom_values_in.rs +++ b/crates/nu-plugin-core/src/util/with_custom_values_in.rs @@ -2,8 +2,6 @@ use nu_protocol::{CustomValue, IntoSpanned, ShellError, Spanned, Value}; /// Do something with all [`CustomValue`]s recursively within a `Value`. This is not limited to /// plugin custom values. -/// -/// `LazyRecord`s will be collected to plain values for completeness. pub fn with_custom_values_in( value: &mut Value, mut f: impl FnMut(Spanned<&mut Box>) -> Result<(), E>, @@ -18,13 +16,6 @@ where // Operate on a CustomValue. f(val.into_spanned(span)) } - // LazyRecord would be a problem for us, since it could return something else the - // next time, and we have to collect it anyway to serialize it. Collect it in place, - // and then use the result - Value::LazyRecord { val, .. } => { - *value = val.collect()?; - Ok(()) - } _ => Ok(()), } }) @@ -33,31 +24,7 @@ where #[test] fn find_custom_values() { use nu_plugin_protocol::test_util::test_plugin_custom_value; - use nu_protocol::{engine::Closure, record, LazyRecord, Span}; - - #[derive(Debug, Clone)] - struct Lazy; - impl<'a> LazyRecord<'a> for Lazy { - fn column_names(&'a self) -> Vec<&'a str> { - vec!["custom", "plain"] - } - - fn get_column_value(&self, column: &str) -> Result { - Ok(match column { - "custom" => Value::test_custom_value(Box::new(test_plugin_custom_value())), - "plain" => Value::test_int(42), - _ => unimplemented!(), - }) - } - - fn span(&self) -> Span { - Span::test_data() - } - - fn clone_value(&self, span: Span) -> Value { - Value::lazy_record(Box::new(self.clone()), span) - } - } + use nu_protocol::{engine::Closure, record}; let mut cv = Value::test_custom_value(Box::new(test_plugin_custom_value())); @@ -73,7 +40,6 @@ fn find_custom_values() { captures: vec![(0, cv.clone()), (1, Value::test_string("foo"))] } ), - "lazy" => Value::test_lazy_record(Box::new(Lazy)), }); // Do with_custom_values_in, and count the number of custom values found @@ -83,7 +49,7 @@ fn find_custom_values() { Ok(()) }) .expect("error"); - assert_eq!(4, found, "found in value"); + assert_eq!(3, found, "found in value"); // Try it on bare custom value too found = 0; diff --git a/crates/nu-plugin-protocol/src/plugin_custom_value/mod.rs b/crates/nu-plugin-protocol/src/plugin_custom_value/mod.rs index e8e83d76d2..ea2b1551c9 100644 --- a/crates/nu-plugin-protocol/src/plugin_custom_value/mod.rs +++ b/crates/nu-plugin-protocol/src/plugin_custom_value/mod.rs @@ -179,11 +179,6 @@ impl PluginCustomValue { Ok(()) } } - // Collect LazyRecord before proceeding - Value::LazyRecord { ref val, .. } => { - *value = val.collect()?; - Ok(()) - } _ => Ok(()), } }) @@ -205,11 +200,6 @@ impl PluginCustomValue { Ok(()) } } - // Collect LazyRecord before proceeding - Value::LazyRecord { ref val, .. } => { - *value = val.collect()?; - Ok(()) - } _ => Ok(()), } }) @@ -224,11 +214,6 @@ impl PluginCustomValue { *value = val.to_base_value(span)?; Ok(()) } - // Collect LazyRecord before proceeding - Value::LazyRecord { ref val, .. } => { - *value = val.collect()?; - Ok(()) - } _ => Ok(()), } }) diff --git a/crates/nu-plugin-test-support/src/plugin_test.rs b/crates/nu-plugin-test-support/src/plugin_test.rs index f4fc85e440..16eb62cf2e 100644 --- a/crates/nu-plugin-test-support/src/plugin_test.rs +++ b/crates/nu-plugin-test-support/src/plugin_test.rs @@ -344,9 +344,6 @@ impl PluginTest { // All equal, and same length Ok(true) } - // Must collect lazy records to compare. - (Value::LazyRecord { val: a_val, .. }, _) => self.value_eq(&a_val.collect()?, b), - (_, Value::LazyRecord { val: b_val, .. }) => self.value_eq(a, &b_val.collect()?), // Fall back to regular eq. _ => Ok(a == b), } diff --git a/crates/nu-plugin/src/plugin/command.rs b/crates/nu-plugin/src/plugin/command.rs index 8678743524..ad8ecd7d9c 100644 --- a/crates/nu-plugin/src/plugin/command.rs +++ b/crates/nu-plugin/src/plugin/command.rs @@ -381,11 +381,6 @@ pub(crate) fn render_examples( plugin.custom_value_to_base_value(engine, val.into_spanned(span))?; Ok::<_, ShellError>(()) } - // Collect LazyRecord before proceeding - Value::LazyRecord { ref val, .. } => { - *value = val.collect()?; - Ok(()) - } _ => Ok(()), } })?; diff --git a/crates/nu-protocol/src/errors/shell_error.rs b/crates/nu-protocol/src/errors/shell_error.rs index 48d2ec88e8..e1d7ade338 100644 --- a/crates/nu-protocol/src/errors/shell_error.rs +++ b/crates/nu-protocol/src/errors/shell_error.rs @@ -1178,16 +1178,6 @@ pub enum ShellError { span: Option, }, - /// An attempt to access a record column failed. - #[error("Access failure: {message}")] - #[diagnostic(code(nu::shell::lazy_record_access_failed))] - LazyRecordAccessFailed { - message: String, - column_name: String, - #[label("Could not access '{column_name}' on this record")] - span: Span, - }, - /// Operation interrupted by user #[error("Operation interrupted by user")] InterruptedByUser { diff --git a/crates/nu-protocol/src/value/lazy_record.rs b/crates/nu-protocol/src/value/lazy_record.rs deleted file mode 100644 index f4f0f93b0a..0000000000 --- a/crates/nu-protocol/src/value/lazy_record.rs +++ /dev/null @@ -1,29 +0,0 @@ -use crate::{Record, ShellError, Span, Value}; -use std::fmt; - -// Trait definition for a lazy record (where columns are evaluated on-demand) -// typetag is needed to make this implement Serialize+Deserialize... even though we should never actually serialize a LazyRecord. -// To serialize a LazyRecord, collect it into a Value::Record with collect() first. -pub trait LazyRecord<'a>: fmt::Debug + Send + Sync { - // All column names - fn column_names(&'a self) -> Vec<&'a str>; - - // Get 1 specific column value - fn get_column_value(&self, column: &str) -> Result; - - fn span(&self) -> Span; - - // Convert the lazy record into a regular Value::Record by collecting all its columns - fn collect(&'a self) -> Result { - self.column_names() - .into_iter() - .map(|col| { - let val = self.get_column_value(col)?; - Ok((col.to_owned(), val)) - }) - .collect::>() - .map(|record| Value::record(record, self.span())) - } - - fn clone_value(&self, span: Span) -> Value; -} diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 64b5abf64d..dbfa93b793 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -4,7 +4,6 @@ mod filesize; mod from; mod from_value; mod glob; -mod lazy_record; mod range; pub mod record; @@ -13,7 +12,6 @@ pub use duration::*; pub use filesize::*; pub use from_value::FromValue; pub use glob::*; -pub use lazy_record::LazyRecord; pub use range::{FloatRange, IntRange, Range}; pub use record::Record; @@ -164,13 +162,6 @@ pub enum Value { #[serde(rename = "span")] internal_span: Span, }, - #[serde(skip)] - LazyRecord { - val: Box LazyRecord<'a>>, - // note: spans are being refactored out of Value - // please use .span() instead of matching this span value - internal_span: Span, - }, } impl Clone for Value { @@ -212,7 +203,6 @@ impl Clone for Value { val: val.clone(), internal_span: *internal_span, }, - Value::LazyRecord { val, internal_span } => val.clone_value(*internal_span), Value::List { vals, internal_span, @@ -672,24 +662,6 @@ impl Value { } } - /// Returns a reference to the inner [`LazyRecord`] trait object or an error if this `Value` is not a lazy record - pub fn as_lazy_record(&self) -> Result<&dyn for<'a> LazyRecord<'a>, ShellError> { - if let Value::LazyRecord { val, .. } = self { - Ok(val.as_ref()) - } else { - self.cant_convert_to("lazy record") - } - } - - /// Unwraps the inner [`LazyRecord`] trait object or returns an error if this `Value` is not a lazy record - pub fn into_lazy_record(self) -> Result LazyRecord<'a>>, ShellError> { - if let Value::LazyRecord { val, .. } = self { - Ok(val) - } else { - self.cant_convert_to("lazy record") - } - } - /// Get the span for the current value pub fn span(&self) -> Span { match self { @@ -709,7 +681,6 @@ impl Value { | Value::Binary { internal_span, .. } | Value::CellPath { internal_span, .. } | Value::Custom { internal_span, .. } - | Value::LazyRecord { internal_span, .. } | Value::Error { internal_span, .. } => *internal_span, } } @@ -727,7 +698,6 @@ impl Value { | Value::String { internal_span, .. } | Value::Glob { internal_span, .. } | Value::Record { internal_span, .. } - | Value::LazyRecord { internal_span, .. } | Value::List { internal_span, .. } | Value::Closure { internal_span, .. } | Value::Nothing { internal_span, .. } @@ -784,10 +754,6 @@ impl Value { None => Type::List(Box::new(Type::Any)), } } - Value::LazyRecord { val, .. } => match val.collect() { - Ok(val) => val.get_type(), - Err(..) => Type::Error, - }, Value::Nothing { .. } => Type::Nothing, Value::Closure { .. } => Type::Closure, Value::Error { .. } => Type::Error, @@ -893,10 +859,6 @@ impl Value { .collect::>() .join(separator) ), - Value::LazyRecord { val, .. } => val - .collect() - .unwrap_or_else(|err| Value::error(err, span)) - .to_expanded_string(separator, config), Value::Closure { val, .. } => format!("", val.block_id), Value::Nothing { .. } => String::new(), Value::Error { error, .. } => format!("{error:?}"), @@ -919,7 +881,6 @@ impl Value { /// - "[list {n} items]" /// - "[record {n} fields]" pub fn to_abbreviated_string(&self, config: &Config) -> String { - let span = self.span(); match self { Value::Date { val, .. } => match &config.datetime_table_format { Some(format) => self.format_datetime(val, format), @@ -945,10 +906,6 @@ impl Value { val.len(), if val.len() == 1 { "" } else { "s" } ), - Value::LazyRecord { val, .. } => val - .collect() - .unwrap_or_else(|err| Value::error(err, span)) - .to_abbreviated_string(config), val => val.to_expanded_string(", ", config), } } @@ -1087,7 +1044,7 @@ impl Value { } // Records (and tables) are the only built-in which support column names, // so only use this message for them. - Value::Record { .. } | Value::LazyRecord { .. } => { + Value::Record { .. } => { return Err(ShellError::TypeMismatch { err_message:"Can't access record values with a row index. Try specifying a column name instead".into(), span: *origin_span, @@ -1137,32 +1094,6 @@ impl Value { }); } } - Value::LazyRecord { val, .. } => { - let columns = val.column_names(); - - if let Some(col) = columns.iter().rev().find(|&col| { - if insensitive { - col.eq_ignore_case(column_name) - } else { - col == column_name - } - }) { - current = val.get_column_value(col)?; - } else if *optional { - return Ok(Value::nothing(*origin_span)); // short-circuit - } else if let Some(suggestion) = did_you_mean(&columns, column_name) { - return Err(ShellError::DidYouMean { - suggestion, - span: *origin_span, - }); - } else { - return Err(ShellError::CantFindColumn { - col_name: column_name.clone(), - span: *origin_span, - src_span: span, - }); - } - } // String access of Lists always means Table access. // Create a List which contains each matching value for contained // records in the source list. @@ -1327,11 +1258,6 @@ impl Value { record.to_mut().push(col_name, new_col); } } - Value::LazyRecord { val, .. } => { - // convert to Record first. - *self = val.collect()?; - self.upsert_data_at_cell_path(cell_path, new_val)?; - } Value::Error { error, .. } => return Err(*error.clone()), v => { return Err(ShellError::CantFindColumn { @@ -1443,11 +1369,6 @@ impl Value { }); } } - Value::LazyRecord { val, .. } => { - // convert to Record first. - *self = val.collect()?; - self.update_data_at_cell_path(cell_path, new_val)?; - } Value::Error { error, .. } => return Err(*error.clone()), v => { return Err(ShellError::CantFindColumn { @@ -1532,11 +1453,6 @@ impl Value { } Ok(()) } - Value::LazyRecord { val, .. } => { - // convert to Record first. - *self = val.collect()?; - self.remove_data_at_cell_path(cell_path) - } v => Err(ShellError::CantFindColumn { col_name: col_name.clone(), span: *span, @@ -1616,11 +1532,6 @@ impl Value { } Ok(()) } - Value::LazyRecord { val, .. } => { - // convert to Record first. - *self = val.collect()?; - self.remove_data_at_cell_path(cell_path) - } v => Err(ShellError::CantFindColumn { col_name: col_name.clone(), span: *span, @@ -1739,11 +1650,6 @@ impl Value { record.to_mut().push(col_name, new_col); } } - Value::LazyRecord { val, .. } => { - // convert to Record first. - *self = val.collect()?; - self.insert_data_at_cell_path(cell_path, new_val, v_span)?; - } other => { return Err(ShellError::UnsupportedInput { msg: "table or record".into(), @@ -1797,8 +1703,6 @@ impl Value { /// /// If the closure returns `Err`, the traversal will stop. /// - /// If collecting lazy records to check them as well is desirable, make sure to do it in your - /// closure. The traversal continues on whatever modifications you make during the closure. /// Captures of closure values are currently visited, as they are values owned by the closure. pub fn recurse_mut( &mut self, @@ -1837,7 +1741,7 @@ impl Value { | Value::Binary { .. } | Value::CellPath { .. } => Ok(()), // These could potentially contain values, but we expect the closure to handle them - Value::LazyRecord { .. } | Value::Custom { .. } => Ok(()), + Value::Custom { .. } => Ok(()), } } @@ -1998,13 +1902,6 @@ impl Value { } } - pub fn lazy_record(val: Box LazyRecord<'a>>, span: Span) -> Value { - Value::LazyRecord { - val, - internal_span: span, - } - } - /// Note: Only use this for test data, *not* live data, as it will point into unknown source /// when used in errors. pub fn test_bool(val: bool) -> Value { @@ -2101,17 +1998,11 @@ impl Value { Value::custom(val, Span::test_data()) } - /// Note: Only use this for test data, *not* live data, as it will point into unknown source - /// when used in errors. - pub fn test_lazy_record(val: Box LazyRecord<'a>>) -> Value { - Value::lazy_record(val, Span::test_data()) - } - /// Note: Only use this for test data, *not* live data, /// as it will point into unknown source when used in errors. /// /// Returns a `Vec` containing one of each value case (`Value::Int`, `Value::String`, etc.) - /// except for `Value::LazyRecord` and `Value::CustomValue`. + /// except for `Value::CustomValue`. pub fn test_values() -> Vec { vec![ Value::test_bool(false), @@ -2127,7 +2018,6 @@ impl Value { Value::test_float(0.0), Value::test_string(String::new()), Value::test_record(Record::new()), - // Value::test_lazy_record(Box::new(todo!())), Value::test_list(Vec::new()), Value::test_closure(Closure { block_id: 0, @@ -2181,7 +2071,6 @@ impl PartialOrd for Value { Value::String { .. } => Some(Ordering::Less), Value::Glob { .. } => Some(Ordering::Less), Value::Record { .. } => Some(Ordering::Less), - Value::LazyRecord { .. } => Some(Ordering::Less), Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), Value::Nothing { .. } => Some(Ordering::Less), @@ -2201,7 +2090,6 @@ impl PartialOrd for Value { Value::String { .. } => Some(Ordering::Less), Value::Glob { .. } => Some(Ordering::Less), Value::Record { .. } => Some(Ordering::Less), - Value::LazyRecord { .. } => Some(Ordering::Less), Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), Value::Nothing { .. } => Some(Ordering::Less), @@ -2221,7 +2109,6 @@ impl PartialOrd for Value { Value::String { .. } => Some(Ordering::Less), Value::Glob { .. } => Some(Ordering::Less), Value::Record { .. } => Some(Ordering::Less), - Value::LazyRecord { .. } => Some(Ordering::Less), Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), Value::Nothing { .. } => Some(Ordering::Less), @@ -2241,7 +2128,6 @@ impl PartialOrd for Value { Value::String { .. } => Some(Ordering::Less), Value::Glob { .. } => Some(Ordering::Less), Value::Record { .. } => Some(Ordering::Less), - Value::LazyRecord { .. } => Some(Ordering::Less), Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), Value::Nothing { .. } => Some(Ordering::Less), @@ -2261,7 +2147,6 @@ impl PartialOrd for Value { Value::String { .. } => Some(Ordering::Less), Value::Glob { .. } => Some(Ordering::Less), Value::Record { .. } => Some(Ordering::Less), - Value::LazyRecord { .. } => Some(Ordering::Less), Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), Value::Nothing { .. } => Some(Ordering::Less), @@ -2281,7 +2166,6 @@ impl PartialOrd for Value { Value::String { .. } => Some(Ordering::Less), Value::Glob { .. } => Some(Ordering::Less), Value::Record { .. } => Some(Ordering::Less), - Value::LazyRecord { .. } => Some(Ordering::Less), Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), Value::Nothing { .. } => Some(Ordering::Less), @@ -2301,7 +2185,6 @@ impl PartialOrd for Value { Value::String { .. } => Some(Ordering::Less), Value::Glob { .. } => Some(Ordering::Less), Value::Record { .. } => Some(Ordering::Less), - Value::LazyRecord { .. } => Some(Ordering::Less), Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), Value::Nothing { .. } => Some(Ordering::Less), @@ -2321,7 +2204,6 @@ impl PartialOrd for Value { Value::String { val: rhs, .. } => lhs.partial_cmp(rhs), Value::Glob { val: rhs, .. } => lhs.partial_cmp(rhs), Value::Record { .. } => Some(Ordering::Less), - Value::LazyRecord { .. } => Some(Ordering::Less), Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), Value::Nothing { .. } => Some(Ordering::Less), @@ -2341,7 +2223,6 @@ impl PartialOrd for Value { Value::String { val: rhs, .. } => lhs.partial_cmp(rhs), Value::Glob { val: rhs, .. } => lhs.partial_cmp(rhs), Value::Record { .. } => Some(Ordering::Less), - Value::LazyRecord { .. } => Some(Ordering::Less), Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), Value::Nothing { .. } => Some(Ordering::Less), @@ -2387,13 +2268,6 @@ impl PartialOrd for Value { // that the shorter sequence is less than the longer one lhs.len().partial_cmp(&rhs.len()) } - Value::LazyRecord { val, .. } => { - if let Ok(rhs) = val.collect() { - self.partial_cmp(&rhs) - } else { - None - } - } Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), Value::Nothing { .. } => Some(Ordering::Less), @@ -2413,7 +2287,6 @@ impl PartialOrd for Value { Value::String { .. } => Some(Ordering::Greater), Value::Glob { .. } => Some(Ordering::Greater), Value::Record { .. } => Some(Ordering::Greater), - Value::LazyRecord { .. } => Some(Ordering::Greater), Value::List { vals: rhs, .. } => lhs.partial_cmp(rhs), Value::Closure { .. } => Some(Ordering::Less), Value::Nothing { .. } => Some(Ordering::Less), @@ -2433,7 +2306,6 @@ impl PartialOrd for Value { Value::String { .. } => Some(Ordering::Greater), Value::Glob { .. } => Some(Ordering::Greater), Value::Record { .. } => Some(Ordering::Greater), - Value::LazyRecord { .. } => Some(Ordering::Greater), Value::List { .. } => Some(Ordering::Greater), Value::Closure { val: rhs, .. } => lhs.block_id.partial_cmp(&rhs.block_id), Value::Nothing { .. } => Some(Ordering::Less), @@ -2453,7 +2325,6 @@ impl PartialOrd for Value { Value::String { .. } => Some(Ordering::Greater), Value::Glob { .. } => Some(Ordering::Greater), Value::Record { .. } => Some(Ordering::Greater), - Value::LazyRecord { .. } => Some(Ordering::Greater), Value::List { .. } => Some(Ordering::Greater), Value::Closure { .. } => Some(Ordering::Greater), Value::Nothing { .. } => Some(Ordering::Equal), @@ -2473,7 +2344,6 @@ impl PartialOrd for Value { Value::String { .. } => Some(Ordering::Greater), Value::Glob { .. } => Some(Ordering::Greater), Value::Record { .. } => Some(Ordering::Greater), - Value::LazyRecord { .. } => Some(Ordering::Greater), Value::List { .. } => Some(Ordering::Greater), Value::Closure { .. } => Some(Ordering::Greater), Value::Nothing { .. } => Some(Ordering::Greater), @@ -2493,7 +2363,6 @@ impl PartialOrd for Value { Value::String { .. } => Some(Ordering::Greater), Value::Glob { .. } => Some(Ordering::Greater), Value::Record { .. } => Some(Ordering::Greater), - Value::LazyRecord { .. } => Some(Ordering::Greater), Value::List { .. } => Some(Ordering::Greater), Value::Closure { .. } => Some(Ordering::Greater), Value::Nothing { .. } => Some(Ordering::Greater), @@ -2513,7 +2382,6 @@ impl PartialOrd for Value { Value::String { .. } => Some(Ordering::Greater), Value::Glob { .. } => Some(Ordering::Greater), Value::Record { .. } => Some(Ordering::Greater), - Value::LazyRecord { .. } => Some(Ordering::Greater), Value::List { .. } => Some(Ordering::Greater), Value::Closure { .. } => Some(Ordering::Greater), Value::Nothing { .. } => Some(Ordering::Greater), @@ -2523,13 +2391,6 @@ impl PartialOrd for Value { Value::Custom { .. } => Some(Ordering::Less), }, (Value::Custom { val: lhs, .. }, rhs) => lhs.partial_cmp(rhs), - (Value::LazyRecord { val, .. }, rhs) => { - if let Ok(val) = val.collect() { - val.partial_cmp(rhs) - } else { - None - } - } } } } diff --git a/crates/nuon/src/to.rs b/crates/nuon/src/to.rs index 8e9ce71f58..8b69f17a87 100644 --- a/crates/nuon/src/to.rs +++ b/crates/nuon/src/to.rs @@ -221,10 +221,6 @@ fn value_to_string( collection.join(&format!(",{sep}{nl}")) )) } - Value::LazyRecord { val, .. } => { - let collected = val.collect()?; - value_to_string(&collected, span, depth + 1, indent) - } // All strings outside data structures are quoted because they are in 'command position' // (could be mistaken for commands by the Nu parser) Value::String { val, .. } => Ok(escape_quote_string(val)), From 944ebac1c213298f1691d785d4d38f01541fbb92 Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Fri, 3 May 2024 02:36:58 +0200 Subject: [PATCH 09/16] Eliminate dead code in `nu-explore` (#12735) # Description Nightly clippy found some unused fields leading me down a rabbit hole of dead code hidden behind `pub` Generally removing any already dead code or premature configurability that is not exposed to the user. # User-Facing Changes None in effect. Removed some options from the `$env.config.explore.hex-dump` record that were only read into a struct but never used and also not validated. --- crates/nu-explore/src/commands/mod.rs | 7 - .../src/views/binary/binary_widget.rs | 121 ++++-------------- crates/nu-explore/src/views/binary/mod.rs | 49 ++----- .../nu-explore/src/views/cursor/xycursor.rs | 30 ----- 4 files changed, 36 insertions(+), 171 deletions(-) diff --git a/crates/nu-explore/src/commands/mod.rs b/crates/nu-explore/src/commands/mod.rs index 89a528a254..141dc25f56 100644 --- a/crates/nu-explore/src/commands/mod.rs +++ b/crates/nu-explore/src/commands/mod.rs @@ -51,10 +51,3 @@ pub trait ViewCommand { value: Option, ) -> Result; } - -#[derive(Debug, Default, Clone)] -pub struct Shortcode { - pub code: &'static str, - pub context: &'static str, - pub description: &'static str, -} diff --git a/crates/nu-explore/src/views/binary/binary_widget.rs b/crates/nu-explore/src/views/binary/binary_widget.rs index c0f9f49e47..39d6e3f547 100644 --- a/crates/nu-explore/src/views/binary/binary_widget.rs +++ b/crates/nu-explore/src/views/binary/binary_widget.rs @@ -4,7 +4,7 @@ use ratatui::{ buffer::Buffer, layout::Rect, text::Span, - widgets::{Paragraph, StatefulWidget, Widget}, + widgets::{Paragraph, Widget}, }; use crate::{ @@ -12,10 +12,6 @@ use crate::{ views::util::{nu_style_to_tui, text_style_to_tui_style}, }; -use super::Layout; - -type OptStyle = Option; - #[derive(Debug, Clone)] pub struct BinaryWidget<'a> { data: &'a [u8], @@ -73,7 +69,7 @@ impl BinarySettings { #[derive(Debug, Default, Clone)] pub struct BinaryStyle { - colors: BinaryStyleColors, + color_index: Option, indent_index: Indent, indent_data: Indent, indent_ascii: Indent, @@ -83,7 +79,7 @@ pub struct BinaryStyle { impl BinaryStyle { pub fn new( - colors: BinaryStyleColors, + color_index: Option, indent_index: Indent, indent_data: Indent, indent_ascii: Indent, @@ -91,7 +87,7 @@ impl BinaryStyle { show_split: bool, ) -> Self { Self { - colors, + color_index, indent_index, indent_data, indent_ascii, @@ -113,61 +109,8 @@ impl Indent { } } -#[derive(Debug, Default, Clone)] -pub struct BinaryStyleColors { - pub split_left: OptStyle, - pub split_right: OptStyle, - pub index: OptStyle, - pub data: SymbolColor, - pub ascii: SymbolColor, -} - -#[derive(Debug, Default, Clone)] -pub struct SymbolColor { - pub default: OptStyle, - pub zero: OptStyle, - pub unknown: OptStyle, -} - -impl SymbolColor { - pub fn new(default: OptStyle, zero: OptStyle, unknown: OptStyle) -> Self { - Self { - default, - zero, - unknown, - } - } -} - -impl BinaryStyleColors { - pub fn new( - index: OptStyle, - data: SymbolColor, - ascii: SymbolColor, - split_left: OptStyle, - split_right: OptStyle, - ) -> Self { - Self { - split_left, - split_right, - index, - data, - ascii, - } - } -} - -#[derive(Debug, Default)] -pub struct BinaryWidgetState { - pub layout_index: Layout, - pub layout_data: Layout, - pub layout_ascii: Layout, -} - -impl StatefulWidget for BinaryWidget<'_> { - type State = BinaryWidgetState; - - fn render(self, area: Rect, buf: &mut Buffer, state: &mut Self::State) { +impl Widget for BinaryWidget<'_> { + fn render(self, area: Rect, buf: &mut Buffer) { let min_width = get_widget_width(&self); if (area.width as usize) < min_width { @@ -178,12 +121,12 @@ impl StatefulWidget for BinaryWidget<'_> { return; } - render_hexdump(area, buf, state, self); + render_hexdump(area, buf, self); } } // todo: indent color -fn render_hexdump(area: Rect, buf: &mut Buffer, _state: &mut BinaryWidgetState, w: BinaryWidget) { +fn render_hexdump(area: Rect, buf: &mut Buffer, w: BinaryWidget) { const MIN_INDEX_SIZE: usize = 8; let show_index = !w.opts.disable_index; @@ -211,7 +154,7 @@ fn render_hexdump(area: Rect, buf: &mut Buffer, _state: &mut BinaryWidgetState, if show_index { x += render_space(buf, x, y, 1, w.style.indent_index.left); - x += render_hex_usize(buf, x, y, address, index_width, false, get_index_style(&w)); + x += render_hex_usize(buf, x, y, address, index_width, get_index_style(&w)); x += render_space(buf, x, y, 1, w.style.indent_index.right); } @@ -251,7 +194,7 @@ fn render_hexdump(area: Rect, buf: &mut Buffer, _state: &mut BinaryWidgetState, if show_index { x += render_space(buf, x, y, 1, w.style.indent_index.left); - x += render_hex_usize(buf, x, y, address, index_width, false, get_index_style(&w)); + x += render_hex_usize(buf, x, y, address, index_width, get_index_style(&w)); x += render_space(buf, x, y, 1, w.style.indent_index.right); } @@ -313,7 +256,7 @@ fn render_segment(buf: &mut Buffer, x: u16, y: u16, line: &[u8], w: &BinaryWidge } let (_, style) = get_segment_char(w, n); - size += render_hex_u8(buf, x + size, y, n, false, style); + size += render_hex_u8(buf, x + size, y, n, style); count -= 1; } @@ -346,7 +289,7 @@ fn render_ascii_line(buf: &mut Buffer, x: u16, y: u16, line: &[u8], w: &BinaryWi size } -fn render_ascii_char(buf: &mut Buffer, x: u16, y: u16, n: char, style: OptStyle) -> u16 { +fn render_ascii_char(buf: &mut Buffer, x: u16, y: u16, n: char, style: Option) -> u16 { let text = n.to_string(); let mut p = Paragraph::new(text); @@ -362,8 +305,8 @@ fn render_ascii_char(buf: &mut Buffer, x: u16, y: u16, n: char, style: OptStyle) 1 } -fn render_hex_u8(buf: &mut Buffer, x: u16, y: u16, n: u8, big: bool, style: OptStyle) -> u16 { - render_hex_usize(buf, x, y, n as usize, 2, big, style) +fn render_hex_u8(buf: &mut Buffer, x: u16, y: u16, n: u8, style: Option) -> u16 { + render_hex_usize(buf, x, y, n as usize, 2, style) } fn render_hex_usize( @@ -372,10 +315,9 @@ fn render_hex_usize( y: u16, n: usize, width: u16, - big: bool, - style: OptStyle, + style: Option, ) -> u16 { - let text = usize_to_hex(n, width as usize, big); + let text = usize_to_hex(n, width as usize); let mut p = Paragraph::new(text); if let Some(style) = style { let style = nu_style_to_tui(style); @@ -389,7 +331,7 @@ fn render_hex_usize( width } -fn get_ascii_char(_w: &BinaryWidget, n: u8) -> (char, OptStyle) { +fn get_ascii_char(_w: &BinaryWidget, n: u8) -> (char, Option) { let (style, c) = categorize_byte(&n); let c = c.unwrap_or(n as char); let style = if style.is_plain() { None } else { Some(style) }; @@ -397,7 +339,7 @@ fn get_ascii_char(_w: &BinaryWidget, n: u8) -> (char, OptStyle) { (c, style) } -fn get_segment_char(_w: &BinaryWidget, n: u8) -> (char, OptStyle) { +fn get_segment_char(_w: &BinaryWidget, n: u8) -> (char, Option) { let (style, c) = categorize_byte(&n); let c = c.unwrap_or(n as char); let style = if style.is_plain() { None } else { Some(style) }; @@ -405,8 +347,8 @@ fn get_segment_char(_w: &BinaryWidget, n: u8) -> (char, OptStyle) { (c, style) } -fn get_index_style(w: &BinaryWidget) -> OptStyle { - w.style.colors.index +fn get_index_style(w: &BinaryWidget) -> Option { + w.style.color_index } fn render_space(buf: &mut Buffer, x: u16, y: u16, height: u16, padding: u16) -> u16 { @@ -443,7 +385,7 @@ fn get_max_index_size(w: &BinaryWidget) -> usize { let line_size = w.opts.count_segments * (w.opts.segment_size * 2); let count_lines = w.data.len() / line_size; let max_index = w.opts.index_offset + count_lines * line_size; - usize_to_hex(max_index, 0, false).len() + usize_to_hex(max_index, 0).len() } fn get_widget_width(w: &BinaryWidget) -> usize { @@ -453,7 +395,7 @@ fn get_widget_width(w: &BinaryWidget) -> usize { let count_lines = w.data.len() / line_size; let max_index = w.opts.index_offset + count_lines * line_size; - let index_size = usize_to_hex(max_index, 0, false).len(); + let index_size = usize_to_hex(max_index, 0).len(); let index_size = index_size.max(MIN_INDEX_SIZE); let data_split_size = w.opts.count_segments.saturating_sub(1) * w.style.indent_segment; @@ -479,17 +421,11 @@ fn get_widget_width(w: &BinaryWidget) -> usize { min_width } -fn usize_to_hex(n: usize, width: usize, big: bool) -> String { +fn usize_to_hex(n: usize, width: usize) -> String { if width == 0 { - match big { - true => format!("{:X}", n), - false => format!("{:x}", n), - } + format!("{:x}", n) } else { - match big { - true => format!("{:0>width$X}", n, width = width), - false => format!("{:0>width$x}", n, width = width), - } + format!("{:0>width$x}", n, width = width) } } @@ -499,9 +435,8 @@ mod tests { #[test] fn test_to_hex() { - assert_eq!(usize_to_hex(1, 2, false), "01"); - assert_eq!(usize_to_hex(16, 2, false), "10"); - assert_eq!(usize_to_hex(29, 2, false), "1d"); - assert_eq!(usize_to_hex(29, 2, true), "1D"); + assert_eq!(usize_to_hex(1, 2), "01"); + assert_eq!(usize_to_hex(16, 2), "10"); + assert_eq!(usize_to_hex(29, 2), "1d"); } } diff --git a/crates/nu-explore/src/views/binary/mod.rs b/crates/nu-explore/src/views/binary/mod.rs index 119c852031..cb4bfd9c58 100644 --- a/crates/nu-explore/src/views/binary/mod.rs +++ b/crates/nu-explore/src/views/binary/mod.rs @@ -19,29 +19,17 @@ use crate::{ util::create_map, }; -use self::binary_widget::{ - BinarySettings, BinaryStyle, BinaryStyleColors, BinaryWidget, BinaryWidgetState, Indent, - SymbolColor, -}; +use self::binary_widget::{BinarySettings, BinaryStyle, BinaryWidget, Indent}; use super::{cursor::XYCursor, Layout, View, ViewConfig}; #[derive(Debug, Clone)] pub struct BinaryView { data: Vec, - mode: Option, cursor: XYCursor, settings: Settings, } -#[allow(dead_code)] // todo: -#[derive(Debug, Clone, Copy)] -enum CursorMode { - Index, - Data, - Ascii, -} - #[derive(Debug, Default, Clone)] struct Settings { opts: BinarySettings, @@ -52,7 +40,6 @@ impl BinaryView { pub fn new(data: Vec) -> Self { Self { data, - mode: None, cursor: XYCursor::default(), settings: Settings::default(), } @@ -61,9 +48,8 @@ impl BinaryView { impl View for BinaryView { fn draw(&mut self, f: &mut Frame, area: Rect, _cfg: ViewConfig<'_>, _layout: &mut Layout) { - let mut state = BinaryWidgetState::default(); let widget = create_binary_widget(self); - f.render_stateful_widget(widget, area, &mut state); + f.render_widget(widget, area); } fn handle_input( @@ -77,7 +63,7 @@ impl View for BinaryView { let result = handle_event_view_mode(self, &key); if matches!(&result, Some(Transition::Ok)) { - let report = create_report(self.mode, self.cursor); + let report = create_report(self.cursor); info.status = Some(report); } @@ -206,21 +192,7 @@ fn settings_from_config(config: &ConfigMap) -> Settings { 0, ), style: BinaryStyle::new( - BinaryStyleColors::new( - colors.get("color_index").cloned(), - SymbolColor::new( - colors.get("color_segment").cloned(), - colors.get("color_segment_zero").cloned(), - colors.get("color_segment_unknown").cloned(), - ), - SymbolColor::new( - colors.get("color_ascii").cloned(), - colors.get("color_ascii_zero").cloned(), - colors.get("color_ascii_unknown").cloned(), - ), - colors.get("color_split_left").cloned(), - colors.get("color_split_right").cloned(), - ), + colors.get("color_index").cloned(), Indent::new( config_get_usize(config, "padding_index_left", 2) as u16, config_get_usize(config, "padding_index_right", 2) as u16, @@ -254,22 +226,17 @@ fn config_get_usize(config: &ConfigMap, key: &str, default: usize) -> usize { .unwrap_or(default) } -fn create_report(mode: Option, cursor: XYCursor) -> Report { +fn create_report(cursor: XYCursor) -> Report { let covered_percent = report_row_position(cursor); let cursor = report_cursor_position(cursor); - let mode = report_mode_name(mode); + let mode = report_mode_name(); let msg = String::new(); Report::new(msg, Severity::Info, mode, cursor, covered_percent) } -fn report_mode_name(cursor: Option) -> String { - match cursor { - Some(CursorMode::Index) => String::from("ADDR"), - Some(CursorMode::Data) => String::from("DUMP"), - Some(CursorMode::Ascii) => String::from("TEXT"), - None => String::from("VIEW"), - } +fn report_mode_name() -> String { + String::from("VIEW") } fn report_row_position(cursor: XYCursor) -> String { diff --git a/crates/nu-explore/src/views/cursor/xycursor.rs b/crates/nu-explore/src/views/cursor/xycursor.rs index 3332ded7c6..82ca9422c8 100644 --- a/crates/nu-explore/src/views/cursor/xycursor.rs +++ b/crates/nu-explore/src/views/cursor/xycursor.rs @@ -32,25 +32,10 @@ impl XYCursor { self.x.index() } - #[allow(dead_code)] - pub fn row_offset(&self) -> usize { - self.y.offset() - } - - #[allow(dead_code)] - pub fn column_limit(&self) -> usize { - self.x.end() - } - pub fn row_limit(&self) -> usize { self.y.end() } - #[allow(dead_code)] - pub fn column_offset(&self) -> usize { - self.x.offset() - } - pub fn row_starts_at(&self) -> usize { self.y.starts_at() } @@ -67,11 +52,6 @@ impl XYCursor { self.x.offset() } - #[allow(dead_code)] - pub fn row_window_size(&self) -> usize { - self.y.window() - } - pub fn column_window_size(&self) -> usize { self.x.window() } @@ -80,11 +60,6 @@ impl XYCursor { self.y.next(1) } - #[allow(dead_code)] - pub fn next_row_by(&mut self, i: usize) -> bool { - self.y.next(i) - } - pub fn next_row_page(&mut self) -> bool { self.y.next_window() } @@ -101,11 +76,6 @@ impl XYCursor { self.y.prev(1) } - #[allow(dead_code)] - pub fn prev_row_by(&mut self, i: usize) -> bool { - self.y.prev(i) - } - pub fn prev_row_page(&mut self) -> bool { self.y.prev_window() } From bc6d934fa130962f9b5017477eaa6e52daea3d8c Mon Sep 17 00:00:00 2001 From: Antoine Stevan <44101798+amtoine@users.noreply.github.com> Date: Fri, 3 May 2024 03:25:19 +0200 Subject: [PATCH 10/16] add support for cell-paths to NUON (#12718) # Description _cell paths_ can be easily serialized back and forth to NUON with the leading `$.` syntax. # User-Facing Changes ```nushell $.foo.bar.0 | to nuon ``` and ```nushell "$.foo.bar.0" | from nuon ``` are now possible # Tests + Formatting a new `cell_path` test has been added to `nuon` # After Submitting --- crates/nuon/src/from.rs | 7 +------ crates/nuon/src/lib.rs | 20 +++++++++++++++++++- crates/nuon/src/to.rs | 7 +------ 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/crates/nuon/src/from.rs b/crates/nuon/src/from.rs index e1ba37b966..457b12d5f9 100644 --- a/crates/nuon/src/from.rs +++ b/crates/nuon/src/from.rs @@ -148,12 +148,7 @@ fn convert_to_value( msg: "calls not supported in nuon".into(), span: expr.span, }), - Expr::CellPath(..) => Err(ShellError::OutsideSpannedLabeledError { - src: original_text.to_string(), - error: "Error when loading".into(), - msg: "subexpressions and cellpaths not supported in nuon".into(), - span: expr.span, - }), + Expr::CellPath(val) => Ok(Value::cell_path(val, span)), Expr::DateTime(dt) => Ok(Value::date(dt, span)), Expr::ExternalCall(..) => Err(ShellError::OutsideSpannedLabeledError { src: original_text.to_string(), diff --git a/crates/nuon/src/lib.rs b/crates/nuon/src/lib.rs index 22153a174f..cb86480580 100644 --- a/crates/nuon/src/lib.rs +++ b/crates/nuon/src/lib.rs @@ -14,7 +14,11 @@ pub use to::ToStyle; #[cfg(test)] mod tests { use chrono::DateTime; - use nu_protocol::{ast::RangeInclusion, engine::Closure, record, IntRange, Range, Span, Value}; + use nu_protocol::{ + ast::{CellPath, PathMember, RangeInclusion}, + engine::Closure, + record, IntRange, Range, Span, Value, + }; use crate::{from_nuon, to_nuon, ToStyle}; @@ -325,6 +329,20 @@ mod tests { ); } + #[test] + fn cell_path() { + nuon_end_to_end( + r#"$.foo.bar.0"#, + Some(Value::test_cell_path(CellPath { + members: vec![ + PathMember::string("foo".to_string(), false, Span::new(2, 5)), + PathMember::string("bar".to_string(), false, Span::new(6, 9)), + PathMember::int(0, false, Span::new(10, 11)), + ], + })), + ); + } + #[test] fn does_not_quote_strings_unnecessarily() { assert_eq!( diff --git a/crates/nuon/src/to.rs b/crates/nuon/src/to.rs index 8b69f17a87..30fe2664a2 100644 --- a/crates/nuon/src/to.rs +++ b/crates/nuon/src/to.rs @@ -97,12 +97,7 @@ fn value_to_string( Ok("false".to_string()) } } - Value::CellPath { .. } => Err(ShellError::UnsupportedInput { - msg: "cell-paths are currently not nuon-compatible".to_string(), - input: "value originates from here".into(), - msg_span: span, - input_span: v.span(), - }), + Value::CellPath { val, .. } => Ok(format!("$.{}", val)), Value::Custom { .. } => Err(ShellError::UnsupportedInput { msg: "custom values are currently not nuon-compatible".to_string(), input: "value originates from here".into(), From 72f3942c377586c7ecee700647d367dc59a30829 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Thu, 2 May 2024 22:31:33 -0700 Subject: [PATCH 11/16] Upgrade to interprocess 2.0.0 (#12729) # Description This fixes #12724. NetBSD confirmed to work with this change. The update also behaves a bit better in some ways - it automatically unlinks and reclaims sockets on Unix, and doesn't try to flush/sync the socket on Windows, so I was able to remove that platform-specific logic. They also have a way to split the socket so I could just use one socket now, but I haven't tried to do that yet. That would be more of a breaking change but I think it's more straightforward. # User-Facing Changes - Hopefully more platforms work # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` --- Cargo.lock | 181 ++---------------- Cargo.toml | 2 +- .../communication_mode/local_socket/mod.rs | 56 ++---- .../src/communication_mode/mod.rs | 61 +++--- crates/nu_plugin_stress_internals/src/main.rs | 39 ++-- 5 files changed, 79 insertions(+), 260 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 170f242459..0e7c9f9203 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -240,30 +240,6 @@ dependencies = [ "wait-timeout", ] -[[package]] -name = "async-channel" -version = "2.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f28243a43d821d11341ab73c80bed182dc015c514b951616cf79bd4af39af0c3" -dependencies = [ - "concurrent-queue", - "event-listener 5.3.0", - "event-listener-strategy 0.5.1", - "futures-core", - "pin-project-lite", -] - -[[package]] -name = "async-lock" -version = "3.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d034b430882f8381900d3fe6f0aaa3ad94f2cb4ac519b429692a1bc2dda4ae7b" -dependencies = [ - "event-listener 4.0.3", - "event-listener-strategy 0.4.0", - "pin-project-lite", -] - [[package]] name = "async-stream" version = "0.3.5" @@ -286,12 +262,6 @@ dependencies = [ "syn 2.0.58", ] -[[package]] -name = "async-task" -version = "4.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fbb36e985947064623dbd357f727af08ffd077f93d696782f3c56365fa2e2799" - [[package]] name = "async-trait" version = "0.1.79" @@ -318,12 +288,6 @@ version = "0.15.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9ae037714f313c1353189ead58ef9eec30a8e8dc101b2622d461418fd59e28a9" -[[package]] -name = "atomic-waker" -version = "1.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1505bd5d3d116872e7271a6d4e16d81d0c8570876c8de68093a09ac269d8aac0" - [[package]] name = "autocfg" version = "1.2.0" @@ -472,22 +436,6 @@ dependencies = [ "generic-array", ] -[[package]] -name = "blocking" -version = "1.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a37913e8dc4ddcc604f0c6d3bf2887c995153af3611de9e23c352b44c1b9118" -dependencies = [ - "async-channel", - "async-lock", - "async-task", - "fastrand", - "futures-io", - "futures-lite", - "piper", - "tracing", -] - [[package]] name = "borsh" version = "1.4.0" @@ -905,15 +853,6 @@ dependencies = [ "static_assertions", ] -[[package]] -name = "concurrent-queue" -version = "2.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d16048cd947b08fa32c24458a22f5dc5e835264f689f4f5653210c69fd107363" -dependencies = [ - "crossbeam-utils", -] - [[package]] name = "condtype" version = "1.3.0" @@ -1437,48 +1376,6 @@ version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b90ca2580b73ab6a1f724b76ca11ab632df820fd6040c336200d2c1df7b3c82c" -[[package]] -name = "event-listener" -version = "4.0.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67b215c49b2b248c855fb73579eb1f4f26c38ffdc12973e20e07b91d78d5646e" -dependencies = [ - "concurrent-queue", - "parking", - "pin-project-lite", -] - -[[package]] -name = "event-listener" -version = "5.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6d9944b8ca13534cdfb2800775f8dd4902ff3fc75a50101466decadfdf322a24" -dependencies = [ - "concurrent-queue", - "parking", - "pin-project-lite", -] - -[[package]] -name = "event-listener-strategy" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "958e4d70b6d5e81971bebec42271ec641e7ff4e170a6fa605f2b8a8b65cb97d3" -dependencies = [ - "event-listener 4.0.3", - "pin-project-lite", -] - -[[package]] -name = "event-listener-strategy" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "332f51cb23d20b0de8458b86580878211da09bcd4503cb579c225b3d124cabb3" -dependencies = [ - "event-listener 5.3.0", - "pin-project-lite", -] - [[package]] name = "fallible-iterator" version = "0.3.0" @@ -1695,16 +1592,6 @@ version = "0.3.30" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a44623e20b9681a318efdd71c299b6b222ed6f231972bfe2f224ebad6311f0c1" -[[package]] -name = "futures-lite" -version = "2.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "52527eb5074e35e9339c6b4e8d12600c7128b68fb25dcb9fa9dec18f7c25f3a5" -dependencies = [ - "futures-core", - "pin-project-lite", -] - [[package]] name = "futures-macro" version = "0.3.30" @@ -2126,30 +2013,16 @@ dependencies = [ [[package]] name = "interprocess" -version = "1.2.1" +version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81f2533f3be42fffe3b5e63b71aeca416c1c3bc33e4e27be018521e76b1f38fb" +checksum = "6d5f0e3c218e7a86a6712fd3adc84672304f9e839402b866685b9117a077c37f" dependencies = [ - "blocking", - "cfg-if", - "futures-core", - "futures-io", - "intmap", "libc", - "once_cell", - "rustc_version", - "spinning", - "thiserror", - "to_method", - "winapi", + "recvmsg", + "widestring", + "windows-sys 0.52.0", ] -[[package]] -name = "intmap" -version = "0.7.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae52f28f45ac2bc96edb7714de995cffc174a395fb0abf5bff453587c980d7b9" - [[package]] name = "inventory" version = "0.3.15" @@ -3830,12 +3703,6 @@ dependencies = [ "unicode-width", ] -[[package]] -name = "parking" -version = "2.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb813b8af86854136c6922af0598d719255ecb2179515e6e7730d468f05c9cae" - [[package]] name = "parking_lot" version = "0.12.1" @@ -4059,17 +3926,6 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" -[[package]] -name = "piper" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "668d31b1c4eba19242f2088b2bf3316b82ca31082a8335764db4e083db7485d4" -dependencies = [ - "atomic-waker", - "fastrand", - "futures-io", -] - [[package]] name = "pkg-config" version = "0.3.30" @@ -4865,6 +4721,12 @@ dependencies = [ "syn 2.0.58", ] +[[package]] +name = "recvmsg" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3edd4d5d42c92f0a659926464d4cce56b562761267ecf0f469d85b7de384175" + [[package]] name = "redox_syscall" version = "0.4.1" @@ -5605,15 +5467,6 @@ dependencies = [ "windows-sys 0.52.0", ] -[[package]] -name = "spinning" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d4f0e86297cad2658d92a707320d87bf4e6ae1050287f51d19b67ef3f153a7b" -dependencies = [ - "lock_api", -] - [[package]] name = "sqlparser" version = "0.39.0" @@ -6051,12 +5904,6 @@ dependencies = [ "regex", ] -[[package]] -name = "to_method" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c7c4ceeeca15c8384bbc3e011dbd8fccb7f068a440b752b7d9b32ceb0ca0e2e8" - [[package]] name = "tokio" version = "1.37.0" @@ -6762,6 +6609,12 @@ dependencies = [ "winsafe", ] +[[package]] +name = "widestring" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7219d36b6eac893fa81e84ebe06485e7dcbb616177469b142df14f1f4deb1311" + [[package]] name = "wild" version = "2.2.1" diff --git a/Cargo.toml b/Cargo.toml index 8382ebec4a..7fbc95a239 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,7 +94,7 @@ heck = "0.5.0" human-date-parser = "0.1.1" indexmap = "2.2" indicatif = "0.17" -interprocess = "1.2.1" +interprocess = "2.0.0" is_executable = "1.0" itertools = "0.12" libc = "0.2" diff --git a/crates/nu-plugin-core/src/communication_mode/local_socket/mod.rs b/crates/nu-plugin-core/src/communication_mode/local_socket/mod.rs index e550892fe1..10f71b8e2b 100644 --- a/crates/nu-plugin-core/src/communication_mode/local_socket/mod.rs +++ b/crates/nu-plugin-core/src/communication_mode/local_socket/mod.rs @@ -1,4 +1,4 @@ -use std::ffi::OsString; +use std::ffi::{OsStr, OsString}; #[cfg(test)] pub(crate) mod tests; @@ -23,6 +23,16 @@ pub fn make_local_socket_name(unique_id: &str) -> OsString { base.into() } +/// Interpret a local socket name for use with `interprocess`. +#[cfg(unix)] +pub fn interpret_local_socket_name( + name: &OsStr, +) -> Result { + use interprocess::local_socket::{GenericFilePath, ToFsName}; + + name.to_fs_name::() +} + /// Generate a name to be used for a local socket specific to this `nu` process, described by the /// given `unique_id`, which should be unique to the purpose of the socket. /// @@ -33,6 +43,16 @@ pub fn make_local_socket_name(unique_id: &str) -> OsString { format!("nu.{}.{}", std::process::id(), unique_id).into() } +/// Interpret a local socket name for use with `interprocess`. +#[cfg(windows)] +pub fn interpret_local_socket_name( + name: &OsStr, +) -> Result { + use interprocess::local_socket::{GenericNamespaced, ToNsName}; + + name.to_ns_name::() +} + /// Determine if the error is just due to the listener not being ready yet in asynchronous mode #[cfg(not(windows))] pub fn is_would_block_err(err: &std::io::Error) -> bool { @@ -48,37 +68,3 @@ pub fn is_would_block_err(err: &std::io::Error) -> bool { e as i64 == windows::Win32::Foundation::ERROR_PIPE_LISTENING.0 as i64 }) } - -/// Wraps the `interprocess` local socket stream for greater compatibility -#[derive(Debug)] -pub struct LocalSocketStream(pub interprocess::local_socket::LocalSocketStream); - -impl From for LocalSocketStream { - fn from(value: interprocess::local_socket::LocalSocketStream) -> Self { - LocalSocketStream(value) - } -} - -impl std::io::Read for LocalSocketStream { - fn read(&mut self, buf: &mut [u8]) -> std::io::Result { - self.0.read(buf) - } -} - -impl std::io::Write for LocalSocketStream { - fn write(&mut self, buf: &[u8]) -> std::io::Result { - self.0.write(buf) - } - - fn flush(&mut self) -> std::io::Result<()> { - // We don't actually flush the underlying socket on Windows. The flush operation on a - // Windows named pipe actually synchronizes with read on the other side, and won't finish - // until the other side is empty. This isn't how most of our other I/O methods work, so we - // just won't do it. The BufWriter above this will have still made a write call with the - // contents of the buffer, which should be good enough. - if cfg!(not(windows)) { - self.0.flush()?; - } - Ok(()) - } -} diff --git a/crates/nu-plugin-core/src/communication_mode/mod.rs b/crates/nu-plugin-core/src/communication_mode/mod.rs index 5d5fd03dd0..1eb72d6ac8 100644 --- a/crates/nu-plugin-core/src/communication_mode/mod.rs +++ b/crates/nu-plugin-core/src/communication_mode/mod.rs @@ -4,9 +4,6 @@ use std::process::{Child, ChildStdin, ChildStdout, Command, Stdio}; use nu_protocol::ShellError; -#[cfg(feature = "local-socket")] -use interprocess::local_socket::LocalSocketListener; - #[cfg(feature = "local-socket")] mod local_socket; @@ -83,15 +80,14 @@ impl CommunicationMode { // For sockets: we need to create the server so that the child won't fail to connect. #[cfg(feature = "local-socket")] CommunicationMode::LocalSocket(name) => { - let listener = LocalSocketListener::bind(name.as_os_str()).map_err(|err| { - ShellError::IOError { + use interprocess::local_socket::ListenerOptions; + + let listener = interpret_local_socket_name(name) + .and_then(|name| ListenerOptions::new().name(name).create_sync()) + .map_err(|err| ShellError::IOError { msg: format!("failed to open socket for plugin: {err}"), - } - })?; - Ok(PreparedServerCommunication::LocalSocket { - name: name.clone(), - listener, - }) + })?; + Ok(PreparedServerCommunication::LocalSocket { listener }) } } } @@ -107,11 +103,13 @@ impl CommunicationMode { // Connect to the specified socket. let get_socket = || { use interprocess::local_socket as ls; - ls::LocalSocketStream::connect(name.as_os_str()) + use ls::traits::Stream; + + interpret_local_socket_name(name) + .and_then(|name| ls::Stream::connect(name)) .map_err(|err| ShellError::IOError { msg: format!("failed to connect to socket: {err}"), }) - .map(LocalSocketStream::from) }; // Reverse order from the server: read in, write out let read_in = get_socket()?; @@ -133,9 +131,7 @@ pub enum PreparedServerCommunication { /// Contains the listener to accept connections on. On Unix, the socket is unlinked on `Drop`. #[cfg(feature = "local-socket")] LocalSocket { - #[cfg_attr(windows, allow(dead_code))] // not used on Windows - name: std::ffi::OsString, - listener: LocalSocketListener, + listener: interprocess::local_socket::Listener, }, } @@ -161,6 +157,9 @@ impl PreparedServerCommunication { } #[cfg(feature = "local-socket")] PreparedServerCommunication::LocalSocket { listener, .. } => { + use interprocess::local_socket::traits::{ + Listener, ListenerNonblockingMode, Stream, + }; use std::time::{Duration, Instant}; const RETRY_PERIOD: Duration = Duration::from_millis(1); @@ -170,13 +169,16 @@ impl PreparedServerCommunication { // Use a loop to try to get two clients from the listener: one for read (the plugin // output) and one for write (the plugin input) - listener.set_nonblocking(true)?; + // + // Be non-blocking on Accept only, so we can timeout. + listener.set_nonblocking(ListenerNonblockingMode::Accept)?; let mut get_socket = || { let mut result = None; while let Ok(None) = child.try_wait() { match listener.accept() { Ok(stream) => { - // Success! But make sure the stream is in blocking mode. + // Success! Ensure the stream is in nonblocking mode though, for + // good measure. Had an issue without this on macOS. stream.set_nonblocking(false)?; result = Some(stream); break; @@ -198,7 +200,7 @@ impl PreparedServerCommunication { } } if let Some(stream) = result { - Ok(LocalSocketStream(stream)) + Ok(stream) } else { // The process may have exited Err(ShellError::PluginFailedToLoad { @@ -215,26 +217,13 @@ impl PreparedServerCommunication { } } -impl Drop for PreparedServerCommunication { - fn drop(&mut self) { - match self { - #[cfg(all(unix, feature = "local-socket"))] - PreparedServerCommunication::LocalSocket { name: path, .. } => { - // Just try to remove the socket file, it's ok if this fails - let _ = std::fs::remove_file(path); - } - _ => (), - } - } -} - /// The required streams for communication from the engine side, i.e. the server in socket terms. pub enum ServerCommunicationIo { Stdio(ChildStdin, ChildStdout), #[cfg(feature = "local-socket")] LocalSocket { - read_out: LocalSocketStream, - write_in: LocalSocketStream, + read_out: interprocess::local_socket::Stream, + write_in: interprocess::local_socket::Stream, }, } @@ -243,7 +232,7 @@ pub enum ClientCommunicationIo { Stdio(Stdin, Stdout), #[cfg(feature = "local-socket")] LocalSocket { - read_in: LocalSocketStream, - write_out: LocalSocketStream, + read_in: interprocess::local_socket::Stream, + write_out: interprocess::local_socket::Stream, }, } diff --git a/crates/nu_plugin_stress_internals/src/main.rs b/crates/nu_plugin_stress_internals/src/main.rs index 2ce2a5536e..bdbe5c8943 100644 --- a/crates/nu_plugin_stress_internals/src/main.rs +++ b/crates/nu_plugin_stress_internals/src/main.rs @@ -1,9 +1,12 @@ use std::{ error::Error, + ffi::OsStr, io::{BufRead, BufReader, Write}, }; -use interprocess::local_socket::LocalSocketStream; +use interprocess::local_socket::{ + self, traits::Stream, GenericFilePath, GenericNamespaced, ToFsName, ToNsName, +}; use serde::Deserialize; use serde_json::{json, Value}; @@ -35,9 +38,6 @@ pub fn main() -> Result<(), Box> { local_socket_path: None, }; - #[allow(unused_mut)] - let mut should_flush = true; - let (mut input, mut output): (Box, Box) = match args.get(1).map(|s| s.as_str()) { Some("--stdio") => ( @@ -49,14 +49,13 @@ pub fn main() -> Result<(), Box> { if opts.refuse_local_socket { std::process::exit(1) } else { - let in_socket = LocalSocketStream::connect(args[2].as_str())?; - let out_socket = LocalSocketStream::connect(args[2].as_str())?; - - #[cfg(windows)] - { - // Flushing on a socket on Windows is weird and waits for the other side - should_flush = false; - } + let name = if cfg!(windows) { + OsStr::new(&args[2]).to_ns_name::()? + } else { + OsStr::new(&args[2]).to_fs_name::()? + }; + let in_socket = local_socket::Stream::connect(name.clone())?; + let out_socket = local_socket::Stream::connect(name)?; (Box::new(BufReader::new(in_socket)), Box::new(out_socket)) } @@ -73,9 +72,7 @@ pub fn main() -> Result<(), Box> { // Send encoding format output.write_all(b"\x04json")?; - if should_flush { - output.flush()?; - } + output.flush()?; // Test exiting without `Hello` if opts.exit_before_hello { @@ -91,7 +88,6 @@ pub fn main() -> Result<(), Box> { // Send `Hello` message write( &mut output, - should_flush, &json!({ "Hello": { "protocol": "nu-plugin", @@ -117,7 +113,7 @@ pub fn main() -> Result<(), Box> { // Parse incoming messages loop { match Value::deserialize(&mut de) { - Ok(message) => handle_message(&mut output, should_flush, &opts, &message)?, + Ok(message) => handle_message(&mut output, &opts, &message)?, Err(err) => { if err.is_eof() { break; @@ -135,7 +131,6 @@ pub fn main() -> Result<(), Box> { fn handle_message( output: &mut impl Write, - should_flush: bool, opts: &Options, message: &Value, ) -> Result<(), Box> { @@ -144,7 +139,6 @@ fn handle_message( if plugin_call.as_str() == Some("Signature") { write( output, - should_flush, &json!({ "CallResponse": [ id, @@ -165,7 +159,6 @@ fn handle_message( }); write( output, - should_flush, &json!({ "CallResponse": [ id, @@ -212,11 +205,9 @@ fn signatures() -> Vec { })] } -fn write(output: &mut impl Write, should_flush: bool, value: &Value) -> Result<(), Box> { +fn write(output: &mut impl Write, value: &Value) -> Result<(), Box> { serde_json::to_writer(&mut *output, value)?; output.write_all(b"\n")?; - if should_flush { - output.flush()?; - } + output.flush()?; Ok(()) } From eff2f1b3b039d8de82891ea594c3970f2bcef5ce Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Fri, 3 May 2024 08:49:44 +0200 Subject: [PATCH 12/16] Update PLATFORM_SUPPORT regarding feature flags (#12741) This was out of date after removing `extra` and moving towards the polars plugin --- devdocs/PLATFORM_SUPPORT.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/devdocs/PLATFORM_SUPPORT.md b/devdocs/PLATFORM_SUPPORT.md index 4f40c073b2..46efdfbe5c 100644 --- a/devdocs/PLATFORM_SUPPORT.md +++ b/devdocs/PLATFORM_SUPPORT.md @@ -34,9 +34,8 @@ We will try to provide builds for all of them but a standard configuration for x We have features of Nushell behind flags that can be passed at compilation time. The design focus of Nushell is primarily expressed by everything accessible without passing additional feature flag. This provides a standard command set and receives the most attention. -Two other feature flags are actively tested but are not guaranteed to express the stable design direction of Nushell: -- `extra` - - This includes commands where we are not convinced that they are ready to be stabilized for 1.0 or popular enough + +One option feature flag is currently tested in CI but contains a feature that may be moved to a plugin: - `dataframe` - This includes dataframe support via `polars` and `arrow2`. Introduces a significant additional compilation and binary size. - Due to the use of SIMD extensions may not be compatible with every minimal architecture. From f32ecc641f7150704527a3d52005ed3ae24ee7f3 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Fri, 3 May 2024 08:35:37 +0000 Subject: [PATCH 13/16] Remove some macros (#12742) # Description Replaces some macros with regular functions or other code. --- crates/nu-cli/src/reedline_config.rs | 200 ++++++--------------- crates/nu-cli/src/syntax_highlight.rs | 205 +++++++++++----------- crates/nu-protocol/src/ast/call.rs | 9 + crates/nu-protocol/src/value/filesize.rs | 41 +++-- tests/shell/pipeline/commands/internal.rs | 67 ++++--- 5 files changed, 220 insertions(+), 302 deletions(-) diff --git a/crates/nu-cli/src/reedline_config.rs b/crates/nu-cli/src/reedline_config.rs index b49dad878c..3f920a00cb 100644 --- a/crates/nu-cli/src/reedline_config.rs +++ b/crates/nu-cli/src/reedline_config.rs @@ -1,6 +1,7 @@ use crate::{menus::NuMenuCompleter, NuHelpCompleter}; use crossterm::event::{KeyCode, KeyModifiers}; use log::trace; +use nu_ansi_term::Style; use nu_color_config::{color_record_to_nustyle, lookup_ansi_color_style}; use nu_engine::eval_block; use nu_parser::parse; @@ -158,21 +159,14 @@ fn add_menu( } } -macro_rules! add_style { - // first arm match add!(1,2), add!(2,3) etc - ($name:expr, $record: expr, $span:expr, $config: expr, $menu:expr, $f:expr) => { - $menu = match extract_value($name, $record, $span) { - Ok(text) => { - let style = match text { - Value::String { val, .. } => lookup_ansi_color_style(&val), - Value::Record { .. } => color_record_to_nustyle(&text), - _ => lookup_ansi_color_style("green"), - }; - $f($menu, style) - } - Err(_) => $menu, - }; - }; +fn get_style(record: &Record, name: &str, span: Span) -> Option