Skip to content

Revert "Show ? for optional entries when displaying CellPaths"#14233

Closed
fdncred wants to merge 1 commit intomainfrom
revert-14042-cellpath-display-optional
Closed

Revert "Show ? for optional entries when displaying CellPaths"#14233
fdncred wants to merge 1 commit intomainfrom
revert-14042-cellpath-display-optional

Conversation

@fdncred
Copy link
Copy Markdown
Contributor

@fdncred fdncred commented Nov 2, 2024 •
edited
Loading

Reverts #14042

Staging this revert because of @hustcer's comment in the original PR.

I think it's actually a breaking change:

Before this PR the following line works:

http get https://api.github.com/repos/nushell/nushell/releases/latest | select -i tag_name assets?.id? | flatten | rename -c {'assets.id': id}

After this PR the above line will broken and should be changed to:

http get https://api.github.com/repos/nushell/nushell/releases/latest | select -i tag_name assets?.id? | flatten | rename -c {'assets?.id?': id}

It's weird that after this change the column names will have a ? suffix if select with -i, it may be unacceptable?:

[[version name]; ['0.1.0' nushell] ['0.1.1' fish] ['0.2.0' zsh]] | select -i version name | columns
 0   version? 
 1   name?

@aionescu
Copy link
Copy Markdown
Contributor

aionescu commented Nov 2, 2024 •
edited
Loading

@fdncred
In my next PR based on this (#14197) I changed the printing used by select to be different than Display, restoring the old behavior (without ?)

@hustcer
Copy link
Copy Markdown
Contributor

hustcer commented Nov 2, 2024

@fdncred In my next PR based on this (#14197) I changed the printing used by select to be different than Display, restoring the old behavior (without ?)

Thanks, I can wait and give it a try

@fdncred fdncred mentioned this pull request Nov 2, 2024
Merged
@fdncred
Copy link
Copy Markdown
Contributor Author

fdncred commented Nov 2, 2024

closing due to #14197

@fdncred fdncred closed this Nov 2, 2024
@hustcer hustcer deleted the revert-14042-cellpath-display-optional branch November 13, 2024 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fdncred @aionescu @hustcer