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

Fix config creation during printing #9353

Merged
merged 4 commits into from Jun 4, 2023
Merged

Conversation

kubouch
Copy link
Contributor

@kubouch kubouch commented Jun 4, 2023

Description

This reverts commit 7f758d3 and re-implements it.

This fixes #9264 without cloning the engine state and using merge_env() by using both the config in permanent state and runtime stack.

Previous solution #9304 required cloning engine state for every print which can get substantial if you have a lot/large values stored in the engine. Also, merge_env() is intended to be used only at the separation between eval and parsing. I hope this approach is cleaner.

This works now

def ctxIssue [] { $env.config.color_config.header = blue; print (ls) }; ctxIssue

User-Facing Changes

Tests + Formatting

After Submitting

@fdncred
Copy link
Collaborator

fdncred commented Jun 4, 2023

I tested this on MacOS and it seems to work great! Good work @kubouch.

Since I'm going to be out for a week, I'd probably go ahead and land it, but it does need to be tested in Windows with PR 519 in nu_scripts as I described in core-team.

@fdncred
Copy link
Collaborator

fdncred commented Jun 4, 2023

Before going out the door, I tested this on Windows and it works great! Thanks!

@kubouch kubouch marked this pull request as ready for review June 4, 2023 19:03
@kubouch kubouch merged commit 82e6873 into nushell:main Jun 4, 2023
16 checks passed
@kubouch kubouch deleted the create-config branch June 4, 2023 19:05
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.

some theme items do not change colors when requested
2 participants