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

nu-explore/ Fix configuration issues #7520

Merged
merged 4 commits into from
Dec 19, 2022
Merged

Conversation

zhiburt
Copy link
Contributor

@zhiburt zhiburt commented Dec 18, 2022

close #7518

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@fdncred
Copy link
Collaborator

fdncred commented Dec 18, 2022

It seems like this is parsing the config much better, however I'm not sure it's using the values. I have this in my config.nu.

  explore: {
    config: {
        cursor_color: 'red'
    }
    table: {
        selected_cell: { bg: 'blue'}
    }
  }

and :config-show shows that values. However, when in tables the background is not blue.
Screenshot 2022-12-18 at 2 01 55 PM

Also, the settings we had from the previous PR, as it relates to border/line colors is now gone. The ls_colors part is there as well as filesize color and datetime color but the lines are not right any longer.
Screenshot 2022-12-18 at 2 02 34 PM
I was expecting something closer to regular ls with my purple lines. The previous PR had the purple lines.
Screenshot 2022-12-18 at 2 03 28 PM

The cursor_color: red looks good. I believe that's only used in the config menus. At least that's the only place i see it like this.
Screenshot 2022-12-18 at 2 05 36 PM
Note that surprisingly these lines are purple here but not in the other screens.

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@zhiburt
Copy link
Contributor Author

zhiburt commented Dec 18, 2022

It seems like this is parsing the config much better, however I'm not sure it's using the values. I have this in my config.nu.

It was actually related to this. I was wrong.

As far as I understand nushell doesn't support records nested more then 2 levels and treats it as strings.
(I might be wrong here).

The menus in the config file serialize pretty deeply. I've never really counted how many levels but here's an example of one that works just fine that's pretty deep.

Good catch.
Must be addressed.

@fdncred
Copy link
Collaborator

fdncred commented Dec 19, 2022

This most recent changes are looking better but still the initial ls | explore is not keeping the line colors configured. These lines should be purple based on my config. The try borders are purple and the config borders are purple, just not regular table views and record views.
Screenshot 2022-12-18 at 10 45 07 PM
I think all the other config settings for values are working though.

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@zhiburt
Copy link
Contributor Author

zhiburt commented Dec 19, 2022

Must be fixed.

Could you rerun CI as it seems to be broken for some other reason?

PS: and sorry for this ping pong comment/fix cycle...

@fdncred
Copy link
Collaborator

fdncred commented Dec 19, 2022

much better, thanks!

@fdncred fdncred merged commit b01f50b into nushell:main Dec 19, 2022
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.

explore config deserialization may not be working
2 participants