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

Add extra options to theme-preview.sh for config #805

Merged
merged 4 commits into from
Nov 18, 2023

Conversation

Rycieos
Copy link
Collaborator

@Rycieos Rycieos commented Nov 11, 2023

The original intent for theme-preview.sh was to have a reference for a static output of a theme. So all runs of the tool for a specific theme would always show the same output.

But a different, completely valid use is to see how a theme would look with your config, or to see how changing your config might change how a theme looks.

Add optional arguments that will allow for loading the user config, input files, or to return to the original functionality of locking the COLUMNS to a set value.

There is still no way to control the mocked data, and there might still be some weird quirks with the imbedded config in the tool conflicting with user config.

Fix the minimal theme using the wrong PS1 variable.
Fix Zsh crash in unfold theme.

Fixes #800

@Rycieos Rycieos added this to the v2.2 milestone Nov 11, 2023
@Rycieos Rycieos requested a review from nojhan November 11, 2023 19:52
Copy link
Collaborator

@nojhan nojhan left a comment

Choose a reason for hiding this comment

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

Awesome, that's going to be useful.

I tested with several themes and presets and it works well, except for the minimal theme, which ends on missing LP_PS1_PREFIX.

tools/theme-preview.sh Show resolved Hide resolved
tools/theme-preview.sh Outdated Show resolved Hide resolved
tools/theme-preview.sh Outdated Show resolved Hide resolved
@Rycieos
Copy link
Collaborator Author

Rycieos commented Nov 12, 2023

it works well, except for the minimal theme, which ends on missing LP_PS1_PREFIX.

So the minimal theme is actually not a theme, but instead a "default theme template" (aka a Liquid Prompt v1 theme). To use such a template, you need to set LP_PS1_FILE to its path.

Likely it should not have been under themes/, since that is just confusing. Maybe we move it to a templates/ directory?

It does leave the UX lacking though. You can load a template like this with an inline config file:

./tools/theme-preview.sh default --config-file <(echo LP_PS1_FILE=themes/minimal/minimal.ps1)

But maybe we add an argument, --template-file, that will do that substitution for you?

@nojhan
Copy link
Collaborator

nojhan commented Nov 12, 2023

But maybe we add an argument, --template-file, that will do that substitution for you?

I guess that would be the clearest option for a naive user, indeed.

The original intent for theme-preview.sh was to have a reference for a
static output of a theme. So all runs of the tool for a specific theme
would always show the same output.

But a different, completely valid use is to see how a theme would look
with your config, or to see how changing your config might change how a
theme looks.

Add optional arguments that will allow for loading the user config,
input files, or to return to the original functionality of locking the
COLUMNS to a set value.

There is still no way to control the mocked data, and there might still
be some weird quirks with the imbedded config in the tool conflicting
with user config.

Fix the minimal theme using the wrong PS1 variable.
Fix Zsh crash in unfold theme.

Fixes #800
@Rycieos Rycieos force-pushed the feature/theme-preview-options branch from 40b6050 to 54cbdcb Compare November 12, 2023 22:13
Rycieos and others added 2 commits November 12, 2023 17:17
Add extra checks and warnings to script.
Also add more docs and docs fixes.
Also add examples for the script to the tests.
Move minimal.ps1 to templates/ directory.

Co-authored-by: nojhan <nojhan@nojhan.net>
@Rycieos Rycieos force-pushed the feature/theme-preview-options branch from 54cbdcb to 6fc4fb8 Compare November 12, 2023 22:18
@Rycieos
Copy link
Collaborator Author

Rycieos commented Nov 12, 2023

This is ready for review. The failed docs lint is because I moved the minimal.ps1 file, and the docs are looking for the new location on the master branch.

@Rycieos Rycieos requested a review from nojhan November 12, 2023 22:26
Copy link
Collaborator

@nojhan nojhan left a comment

Choose a reason for hiding this comment

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

Looks good, just two minor (and optional) questions.

tools/theme-preview.sh Outdated Show resolved Hide resolved
tools/theme-preview.sh Show resolved Hide resolved
Also clarify that config presets are a valid option.
@Rycieos Rycieos requested a review from nojhan November 16, 2023 02:54
@Rycieos Rycieos merged commit e1a133b into master Nov 18, 2023
8 of 9 checks passed
@Rycieos Rycieos deleted the feature/theme-preview-options branch November 18, 2023 18:28
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.

theme-preview fails when sourcing preset
2 participants