Skip to content

Commit 5d98a72

Browse files
WindSoilderamtoine
andauthored
Deprecate --flag: bool in custom command (nushell#11365)
# Description While nushell#11057 is merged, it's hard to tell the difference between `--flag: bool` and `--flag`, and it makes user hard to read custom commands' signature, and hard to use them correctly. After discussion, I think we can deprecate `--flag: bool` usage, and encourage using `--flag` instead. # User-Facing Changes The following code will raise warning message, but don't stop from running. ```nushell ❯ def florb [--dry-run: bool, --another-flag] { "aaa" }; florb Error: × Deprecated: --flag: bool ╭─[entry #7:1:1] 1 │ def florb [--dry-run: bool, --another-flag] { "aaa" }; florb · ──┬─ · ╰── `--flag: bool` is deprecated. Please use `--flag` instead, more info: https://www.nushell.sh/book/custom_commands.html ╰──── aaa ``` cc @kubouch # Tests + Formatting Done # After Submitting - [ ] Add more information under https://www.nushell.sh/book/custom_commands.html to indicate `--dry-run: bool` is not allowed, - [ ] remove `: bool` from custom commands between 0.89 and 0.90 --------- Co-authored-by: Antoine Stevan <[email protected]>
1 parent 109f629 commit 5d98a72

File tree

7 files changed

+56
-3
lines changed

7 files changed

+56
-3
lines changed

crates/nu-cli/src/eval_cmds.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ pub fn evaluate_commands(
3535
let mut working_set = StateWorkingSet::new(engine_state);
3636

3737
let output = parse(&mut working_set, None, commands.item.as_bytes(), false);
38+
if let Some(warning) = working_set.parse_warnings.first() {
39+
report_error(&working_set, warning);
40+
}
41+
3842
if let Some(err) = working_set.parse_errors.first() {
3943
report_error(&working_set, err);
4044

crates/nu-cli/src/util.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,10 @@ pub fn eval_source(
220220
source,
221221
false,
222222
);
223+
if let Some(warning) = working_set.parse_warnings.first() {
224+
report_error(&working_set, warning);
225+
}
226+
223227
if let Some(err) = working_set.parse_errors.first() {
224228
set_last_exit_code(stack, 1);
225229
report_error(&working_set, err);

crates/nu-parser/src/parser.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ use nu_protocol::{
1818
},
1919
engine::StateWorkingSet,
2020
eval_const::eval_constant,
21-
span, BlockId, DidYouMean, Flag, ParseError, PositionalArg, Signature, Span, Spanned,
22-
SyntaxShape, Type, Unit, VarId, ENV_VARIABLE_ID, IN_VARIABLE_ID,
21+
span, BlockId, DidYouMean, Flag, ParseError, ParseWarning, PositionalArg, Signature, Span,
22+
Spanned, SyntaxShape, Type, Unit, VarId, ENV_VARIABLE_ID, IN_VARIABLE_ID,
2323
};
2424

2525
use crate::parse_keywords::{
@@ -3520,6 +3520,13 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
35203520
type_annotated,
35213521
} => {
35223522
working_set.set_variable_type(var_id.expect("internal error: all custom parameters must have var_ids"), syntax_shape.to_type());
3523+
if syntax_shape == SyntaxShape::Boolean {
3524+
working_set.warning(ParseWarning::DeprecatedWarning(
3525+
"--flag: bool".to_string(),
3526+
"--flag".to_string(),
3527+
span,
3528+
));
3529+
}
35233530
*arg = Some(syntax_shape);
35243531
*type_annotated = true;
35253532
}

crates/nu-protocol/src/engine/state_working_set.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::ast::Block;
66
use crate::{
77
BlockId, Config, DeclId, FileId, Module, ModuleId, Span, Type, VarId, Variable, VirtualPathId,
88
};
9-
use crate::{Category, ParseError, Value};
9+
use crate::{Category, ParseError, ParseWarning, Value};
1010
use core::panic;
1111
use std::collections::{HashMap, HashSet};
1212
use std::path::PathBuf;
@@ -27,6 +27,7 @@ pub struct StateWorkingSet<'a> {
2727
/// Whether or not predeclarations are searched when looking up a command (used with aliases)
2828
pub search_predecls: bool,
2929
pub parse_errors: Vec<ParseError>,
30+
pub parse_warnings: Vec<ParseWarning>,
3031
}
3132

3233
impl<'a> StateWorkingSet<'a> {
@@ -39,6 +40,7 @@ impl<'a> StateWorkingSet<'a> {
3940
parsed_module_files: vec![],
4041
search_predecls: true,
4142
parse_errors: vec![],
43+
parse_warnings: vec![],
4244
}
4345
}
4446

@@ -50,6 +52,10 @@ impl<'a> StateWorkingSet<'a> {
5052
self.parse_errors.push(parse_error)
5153
}
5254

55+
pub fn warning(&mut self, parse_warning: ParseWarning) {
56+
self.parse_warnings.push(parse_warning)
57+
}
58+
5359
pub fn num_files(&self) -> usize {
5460
self.delta.num_files() + self.permanent_state.num_files()
5561
}

crates/nu-protocol/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ mod id;
1212
mod lev_distance;
1313
mod module;
1414
mod parse_error;
15+
mod parse_warning;
1516
mod pipeline_data;
1617
#[cfg(feature = "plugin")]
1718
mod plugin_signature;
@@ -35,6 +36,7 @@ pub use id::*;
3536
pub use lev_distance::levenshtein_distance;
3637
pub use module::*;
3738
pub use parse_error::{DidYouMean, ParseError};
39+
pub use parse_warning::ParseWarning;
3840
pub use pipeline_data::*;
3941
#[cfg(feature = "plugin")]
4042
pub use plugin_signature::*;
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
use crate::Span;
2+
use miette::Diagnostic;
3+
use serde::{Deserialize, Serialize};
4+
use thiserror::Error;
5+
6+
#[derive(Clone, Debug, Error, Diagnostic, Serialize, Deserialize)]
7+
pub enum ParseWarning {
8+
#[error("Deprecated: {0}")]
9+
DeprecatedWarning(
10+
String,
11+
String,
12+
#[label = "`{0}` is deprecated and will be removed in 0.90. Please use `{1}` instead, more info: https://www.nushell.sh/book/custom_commands.html"]
13+
Span,
14+
),
15+
}
16+
17+
impl ParseWarning {
18+
pub fn span(&self) -> Span {
19+
match self {
20+
ParseWarning::DeprecatedWarning(_, _, s) => *s,
21+
}
22+
}
23+
}

src/tests/test_custom_commands.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,13 @@ fn custom_flag2() -> TestResult {
146146
)
147147
}
148148

149+
#[test]
150+
fn deprecated_boolean_flag() {
151+
let actual = nu!(r#"def florb [--dry-run: bool, --another-flag] { "aaa" }; florb"#);
152+
assert!(actual.err.contains("Deprecated"));
153+
assert_eq!(actual.out, "aaa");
154+
}
155+
149156
#[test]
150157
fn simple_var_closing() -> TestResult {
151158
run_test("let $x = 10; def foo [] { $x }; foo", "10")

0 commit comments

Comments
 (0)