Fix nufmt removing invalid default values for named flags

[iibabyy]

Description of changes

This PR fixes a bug where nufmt removes the default value of a named flag when that value can't be resolved (for example, an undefined variable).

While fixing it, I found that signature_default_from_source broke on several cases.
Instead of patching them one by one, I replaced it with a small state machine that walks the tokens the same way as the nushell parser (nu_parser::parse_signatures::parse_signature_helper).

Relevant Issues

Fixes #228

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[fdncred]

Thanks for tracking this down. These are notes from my clanker. Let me know what you think.

The -/-- prefix change does fix #228 (I checked with the repro from the issue), and it also fixes flag defaults that main was dropping, like --x: duration = 1sec (main printed = sec) and closure defaults (main dropped them).

But the fix also turns on source-text recovery for every flag default, and together with the new whitespace break that introduces regressions that produce invalid output (details inline, all checked against main vs this branch):

1. The new test fixture has no default value and passes unchanged on main, so it doesn't cover #228.
2. Backtick-string defaults with spaces are truncated (`hello world` becomes `hello). This affects positional parameters too.
3. Raw-string flag defaults are cut at # (r#'abc'# becomes r).
4. A flag name inside another flag's string default gets matched, which corrupts output.

Suggested direction: keep using the AST default span when default_value is Some, and only scan source text when it's None.

Minor cleanup: since scan_default_expr_end now breaks on any whitespace, the "Trim trailing whitespace from the default" loop after it (L1455) can never trim anything and can go. Also, the fixture name doesn't follow the *_issueNNN convention used by the other regression fixtures (e.g. ..._issue228), and it probably doesn't belong in the commands_definitions construct list in run_ground_truth_tests.nu.

[fdncred]

This fixture has no default value, so it doesn't exercise #228. --my_var: any has no = ..., and the const is named var while the body references $my_var. I ran input -> expected through the main binary and it already produces identical output, so both new tests pass without the fix. The repro from the issue would be:

const my_var = $unexisting
def cmd [--var: any = $my_var] { }

It would also help to add cases for the regressions noted in the other comments (backtick string, raw string, flag name inside a string default).

[fdncred]

Stopping at any top-level whitespace truncates defaults that have unquoted spaces, because the scanner only knows about " and ' quotes. Backtick strings are the common case:

def cmd [--var: string = `hello world`] {}
# main: def cmd [--var: string = `hello world`] { }
# PR:   def cmd [--var: string = `hello] { }

def cmd [x: string = `a b`, y = "q"] {}
# main: def cmd [x: string = `a b`, y: string = "q"] { }
# PR:   def cmd [x: string = `a, y: string = "q"] { }

This hits positional parameters too, not only flags, and the output doesn't parse. Backticks (and raw strings, see below) need to be treated as quoted regions before whitespace can end the default.

[fdncred]

Now that flag source recovery actually finds --name, every flag default goes through this scanner. The # break cuts raw strings apart:

def cmd [--var: string = r#'abc'#] {}
# main: def cmd [--var: string = r#'abc'#] { }
# PR:   def cmd [--var: string = r] { }

Positional defaults already had this bug on main. This PR extends it to flags, which used to take the exact AST span. r#'...'# needs to be recognized before the comment check. Raw strings that contain ' would also end in_single too early.

[fdncred]

find_identifier doesn't know about quotes, and now that it accepts a -- prefix it matches flag names that appear inside an earlier flag's string default:

def cmd [--msg: string = "run --force = yes", --force: string = "no"] {}
# main: unchanged
# PR:   def cmd [--msg: string = "run --force = yes", --force: string = yes", --force: string = "no"] { }

It's contrived, but it corrupts output silently. Skipping quoted regions during the search (the scanner already tracks them) would prevent it.

Altitude: in format_signature, the flag branch (around L1079-1084) prefers source_default over flag.default_value for every flag. Before this PR that path never matched a flag, so flags always used the exact AST span. Now all flag defaults, including ones that resolve fine, go through the hand-rolled text scanner, and that exposes the regressions above. A smaller and safer fix for #228: use the AST span when default_value is Some, and only fall back to source scanning when it's None (the unresolvable case). A related point: teaching the generic find_identifier about -/-- prefixes duplicates the ends_with(b"--") || ends_with(b"-") check in the caller. Searching for --{name} / -{name} directly in the flag case would keep find_identifier a plain identifier matcher. Tokenizing the signature with nu_parser::lex instead of the byte scanner would handle backticks, raw strings and comments correctly.

[iibabyy]

Thanks for the review !

After playing a bit with the signature formatting, I ran into other bugs (I added some test files for them) that all came down to how we were doing the lexing in signature_default_from_source.
Instead of fixing the little parts one by one, I looked into whether we could reuse the nushell lexer or parser.
After a few tries, replicating the nushell parser's signature state machine turned out to be the most reliable and simplest approach.

About the suggestion to prioritize the AST when the default is Some(...): doing so would be a problem if the AST holds a resolved value (#204), so I kept prioritizing signature_default_from_source now that it's reliable.

I found other bugs along the way (#230), but I wanted to keep this PR focused on signature_default_from_source. I might create a PR in a few days to fix them if it's still needed.

[fdncred]

ok, i'm fine with this. it should've been using the nushell lexer/parser anyway. Thanks!