Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reuse Closure type in Value::Closure #10894

Merged
merged 1 commit into from Oct 30, 2023
Merged

Conversation

IanManske
Copy link
Member

Description

Reuses the existing Closure type in Value::Closure. This will help with the span refactoring for Value. Additionally, this allows us to more easily box or unbox the Closure case should we chose to do so in the future.

User-Facing Changes

Breaking API change for nu_protocol.

@IanManske
Copy link
Member Author

Just realized I never actually got around to opening a PR for this 😅.
Of course, this can wait until after the next release if necessary.

@fdncred
Copy link
Collaborator

fdncred commented Oct 30, 2023

No comments on the PR yet but I'm glad to see PRs from you again. Welcome back!

@IanManske
Copy link
Member Author

Thanks! I should have more time now to spend on nushell. (Back in business!)

Copy link
Member

@sholderbach sholderbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This looks all good to me.

The prexisting Value::as_block raised an eyebrow for me for a moment but I think this is sane in your context.

@sholderbach sholderbach merged commit 72cb4b6 into nushell:main Oct 30, 2023
19 checks passed
@IanManske IanManske deleted the reuse-closure branch October 30, 2023 23:43
hardfau1t pushed a commit to hardfau1t/nushell that referenced this pull request Dec 14, 2023
# Description
Reuses the existing `Closure` type in `Value::Closure`. This will help
with the span refactoring for `Value`. Additionally, this allows us to
more easily box or unbox the `Closure` case should we chose to do so in
the future.

# User-Facing Changes
Breaking API change for `nu_protocol`.
dmatos2012 pushed a commit to dmatos2012/nushell that referenced this pull request Feb 20, 2024
# Description
Reuses the existing `Closure` type in `Value::Closure`. This will help
with the span refactoring for `Value`. Additionally, this allows us to
more easily box or unbox the `Closure` case should we chose to do so in
the future.

# User-Facing Changes
Breaking API change for `nu_protocol`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants