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

feat(presets): adds config-from-doc and several presets #772

Merged
merged 2 commits into from Jul 3, 2023

Conversation

nojhan
Copy link
Collaborator

@nojhan nojhan commented Mar 3, 2023

Features:

  • Adds the config-from-doc.sh script, to automatically generate a default "liquidpromptrc" config file from the documentation (fix Up-to-date presets examples #755).
  • Adds presets for color schemes:
    • tritanopia (blue-yellow blindness),
    • protanopia-deuteranopia (red-green blindness),
    • 256 colors, dark and light schemes.
  • Adds presets for "icons":
    • more-unicode,
    • nerd-font.
  • Adds more configuration variables:
    • Adds LP_COLOR_OS_*.
    • Set LP_MARK_OS_SEP to the same separator than the CMake's one by default (more consistent).
    • Adds LP_ENABLE_CMAKE_COLORS for enabling auto color based on hash.
    • Adds LP_COLOR_CMAKE_* variables when one do not want hash colors.
    • Adds LP_COLOR_SCLS

Fix along the way:

  • Load config at each prompt activation in theme-preview.sh, to allow preview of presets using public functions (i.e. lp_terminal_format).
  • Reorder "dev env" sections to be more logical.
  • Uses exit code 2 on tools scripts, which is the convention for usage errors.
  • update debian pkg
  • fix doc about OS and CMake.

Technical checklist:

  • code follows our shell standards:
    • correct use of IFS
    • careful quoting
    • cautious array access
    • portable array indexing with _LP_FIRST_INDEX
    • printing a "%" character is done with _LP_PERCENT
    • functions/variable naming conventions
    • functions have local variables
    • data functions have optimization guards (early exits)
    • subshells are avoided as much as possible
    • string substitutions may be done differently in Bash and Zsh (use _LP_SHELL_*)
  • tests and checks have been added, ran, and their warnings fixed:
    • unit tests have been updated (see tests/test_*.sh files)
    • ran test.sh
    • ran shellcheck.sh (requires shellcheck).
  • documentation have been updated accordingly:
    • functions and attributes are documented in alphabetical order
    • new sections are documented in the default theme
    • tag .. versionadded:: X.Y or .. versionchanged:: Y.Z
    • functions signatures have arguments, returned code, and set value(s)
    • attributes have types and defaults
    • ran docs/docs-lint.sh (requires Python 3 and requirements.txt
      installed (cd docs/; python3 -m venv venv; . venv/bin/activate; pip install -r requirements.txt))

Add two presets:
- more-unicode
- nerd-font

Add four color schemes:
- 256 colors for dark themes.
- 256 colors for light themes.
- red-green blindnesses.
- blue-yellow blindness.

Doc:
- mention presets in themes and config.
- fix doc about OS and CMake.

Misc:
- Also uses exit code 2 on tools scripts, which is the convention for usage errors.
- update debian pkg
@nojhan nojhan self-assigned this Mar 3, 2023
@nojhan nojhan added the enhancement Feature request label Mar 3, 2023
@nojhan nojhan added this to the v2.2 milestone Mar 3, 2023
Copy link
Collaborator

@Rycieos Rycieos left a comment

Choose a reason for hiding this comment

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

Looking pretty good mostly.

docs/config.rst Show resolved Hide resolved
docs/theme.rst Show resolved Hide resolved
liquidprompt Show resolved Hide resolved
@nojhan nojhan marked this pull request as ready for review June 27, 2023 19:22
@nojhan nojhan requested a review from Rycieos June 27, 2023 19:22
@Rycieos Rycieos merged commit a36a2fd into master Jul 3, 2023
@Rycieos Rycieos deleted the feat/presets branch July 3, 2023 15:15
@augmentedfourth
Copy link
Contributor

This broke the Homebrew formula, which copies liquidprompt-dist from the root of the repo into /usr/local/share.

@Rycieos Would you prefer the Homebrew installation to generate the -dist file and provide it as an example (as it always has), or give users a link to the documentation here on GitHub? I can do the documentation link pretty easily, but it would take a while for me to find time to make Homebrew generate and deploy the default config file.

@augmentedfourth
Copy link
Contributor

Proposed Homebrew documentation change if I go with the latter (simpler) version:

-      A sample file you may copy and modify has been installed to
-        #{HOMEBREW_PREFIX}/share/liquidpromptrc-dist
+      Configuration documentation can be found on GitHub:
+        https://github.com/nojhan/liquidprompt/blob/master/docs/config.rst

@augmentedfourth
Copy link
Contributor

Maybe the appropriate documentation link should be this one?

@Rycieos
Copy link
Collaborator

Rycieos commented Jul 16, 2023

I would rather see links to documentation, as it will be the most helpful. You could also add a bit about the generation script and how to use it since some might find that useful.

Yes, that is the correct link for the stable release, but for those installing from the master branch, it would be nice to link the "latest" version instead: https://liquidprompt.readthedocs.io/en/latest/config.html. If you could have the documentation differ based on which version they install that would be great.

@augmentedfourth
Copy link
Contributor

augmentedfourth commented Jul 16, 2023

OK. I've set up a commit where the regular install will output:

If you'd like to reconfigure options, you may do so in ~/.liquidpromptrc.
Configuration documentation can be found here:
  https://liquidprompt.readthedocs.io/en/stable/config.html

And HEAD/master install will output:

If you'd like to reconfigure options, you may do so in ~/.liquidpromptrc.
Configuration documentation (for the HEAD version) can be found here:
  https://liquidprompt.readthedocs.io/en/latest/config.html

I will include a message about generating a default config file once there's a release that includes the tool. Adding that message to just master and then adding it in all situations later was more complex than I'd like.

Homebrew/homebrew-core#136761

@augmentedfourth
Copy link
Contributor

Argh. It looks like the current generation of Homebrew maintainers are trying to stop hosting other projects' documentation and only use their "caveats" section for details that are specific to the Homebrew distribution.

"most people do not read the caveats and even less people will if we start using it for all kinds of documentation."

They're asking me to remove that readthedocs link entirely. I might be able to re-add the generated sample config file in the future once the tool exists in an official release.

I'd prefer to do nothing and just wait for the tool to exist everywhere, but the lack of the sample config file is breaking Homebrew HEAD installation with the current formula.

@Rycieos
Copy link
Collaborator

Rycieos commented Jul 19, 2023

Yeah that's not ideal. Just remove the install of the configfile stub for now?

@augmentedfourth
Copy link
Contributor

Yeah that's not ideal. Just remove the install of the configfile stub for now?

That's my plan. I just hope we don't have any problems reintroducing it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Up-to-date presets examples
3 participants