Optional members in cell paths: Attempt 2

[rgwood]

This is a follow up from #7540. Please provide feedback if you have the time!

Summary

This PR lets you use ? to indicate that a member in a cell path is optional and Nushell should return null if that member cannot be accessed.

Unlike the previous PR, ? is now a postfix modifier for cell path members. A cell path of .foo?.bar means that foo is optional and bar is not.

? does not suppress all errors; it is intended to help in situations where data has "holes", i.e. the data types are correct but something is missing. Type mismatches (like trying to do a string path access on a date) will still fail.

Record Examples

{ foo: 123 }.foo # returns 123

{ foo: 123 }.bar # errors
{ foo: 123 }.bar? # returns null

{ foo: 123 } | get bar # errors
{ foo: 123 } | get bar? # returns null

{ foo: 123 }.bar.baz # errors
{ foo: 123 }.bar?.baz # errors because `baz` is not present on the result from `bar?`
{ foo: 123 }.bar.baz? # errors
{ foo: 123 }.bar?.baz? # returns null

List Examples

〉[{foo: 1} {foo: 2} {}].foo
Error: nu::shell::column_not_found

  × Cannot find column
   ╭─[entry #30:1:1]
 1 │ [{foo: 1} {foo: 2} {}].foo
   ·                    ─┬  ─┬─
   ·                     │   ╰── cannot find column 'foo'
   ·                     ╰── value originates here
   ╰────
〉[{foo: 1} {foo: 2} {}].foo?
╭───┬───╮
│ 0 │ 1 │
│ 1 │ 2 │
│ 2 │   │
╰───┴───╯
〉[{foo: 1} {foo: 2} {}].foo?.2 | describe
nothing

〉[a b c].4? | describe
nothing

〉[{foo: 1} {foo: 2} {}] | where foo? == 1
╭───┬─────╮
│ # │ foo │
├───┼─────┤
│ 0 │   1 │
╰───┴─────╯

Breaking changes

1. Column names with ? in them now need to be quoted.
2. The -i/--ignore-errors flag has been removed from get and select
   1. After this PR, most get error handling can be done with ? and/or try/catch.
3. Cell path accesses like this no longer work without a ?:

〉[{a:1 b:2} {a:3}].b.0
2

We had some clever code that was able to recognize that since we only want row 0, it's OK if other rows are missing column b. I removed that because it's tricky to maintain, and now that query needs to be written like:

〉[{a:1 b:2} {a:3}].b?.0
2

I think the regression is acceptable for now. I plan to do more work in the future to enable streaming of cell path accesses, and when that happens I'll be able to make .b.0 work again.

[codecov]

Codecov Report

Merging #8379 (8c7f501) into main (aa876ce) will decrease coverage by 0.03%.
The diff coverage is 69.91%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8379      +/-   ##
==========================================
- Coverage   68.06%   68.04%   -0.03%     
==========================================
  Files         621      621              
  Lines       99885    99943      +58     
==========================================
+ Hits        67988    68002      +14     
- Misses      31897    31941      +44     

Impacted Files	Coverage Δ
crates/nu-command/src/debug/inspect_table.rs	0.00% <0.00%> (ø)
crates/nu-explore/src/nu_common/table.rs	0.00% <0.00%> (ø)
crates/nu-explore/src/nu_common/value.rs	0.00% <0.00%> (ø)
crates/nu-parser/src/parse_keywords.rs	68.51% <0.00%> (-0.05%)	⬇️
crates/nu-command/src/filters/drop/column.rs	80.00% <33.33%> (+2.15%)	⬆️
crates/nu-protocol/src/ast/cell_path.rs	53.19% <35.71%> (-23.00%)	⬇️
crates/nu-protocol/src/value/from_value.rs	34.97% <50.00%> (+0.07%)	⬆️
crates/nu-protocol/src/value/mod.rs	56.29% <59.64%> (-0.28%)	⬇️
crates/nu-cli/src/repl.rs	24.35% <88.57%> (ø)
crates/nu-parser/src/parser.rs	83.26% <91.52%> (+0.05%)	⬆️
... and 15 more

... and 1 file with indirect coverage changes

[fdncred]

Just to be clear, the reason you're suggesting we remove --ignore-errors/-i is because:

These produce identical results

{ foo: 123 } | get bar?
{ foo: 123 } | get -i bar

and these produce identical results

[{a:1 b:2} {a:3}] | select b?
[{a:1 b:2} {a:3}] | select -i b

If that is correct, then I'd propose dropping the -i in this PR, and maybe have a deprecate message on -i?

[rgwood]

Right, ? can replace -i in most situations. For others, try/catch can be used. I’ll remove -i

[rgwood]

On second thought... removing -i is going to be hard, it's used in default_env.nu 😞

let home = ($env | get -i (if $nu.os-info.name == "windows" { "USERPROFILE" } else { "HOME" }) | into string)

Removing the flag will break people's env.nu. Maybe we should leave it around for now.

[rgwood]

Oh! I just noticed that the get -i was added to default_env.nu after the 0.76 release: #8173

If I remove it before the 0.77 release, that will make future deprecation of -i much easier. Will do that in another PR.

[rgwood]

I've removed the -i flag from get and select. Hopefully that won't make too many people angry.

[webbedspace]

Thanks for revisiting this feature!

[rgwood]

I asked in Discord and several people voted to merge this. Let's give it a try!