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+error meaning #745

Merged
merged 1 commit into from Oct 27, 2022
Merged

Feat+error meaning #745

merged 1 commit into from Oct 27, 2022

Conversation

nojhan
Copy link
Collaborator

@nojhan nojhan commented Oct 16, 2022

Basically displays the (guessed) meaning of the last error code (feature request #729).

Separated in two commits, so that you may allow a more or less granular level of details.

First commit: adds the core feature.

  • Use a set of reasonably common error codes on POSIX systems
    (but is LP used out of a POSIX env?).
  • Use the color than lp_error (namely LP_COLOR_ERR).
  • Use a separated LP_ENABLE_ERROR_MEANING.
  • Put in the default theme right after the error code.
  • Code would be very much simpler/configurable with Bash 4's associative arrays,
    but here use a case switch with non-configurable error meanings.

Second commit: let the user decide the level of guess for error meaning.

  • Adds LP_ENABLE_ERROR_MEANING_EXTENDED to allow for a larger set of error codes.

@nojhan nojhan added this to the v2.2 milestone Oct 16, 2022
@nojhan nojhan requested a review from Rycieos October 16, 2022 18:34
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.

Looks good in general, just a few issues.

docs/config.rst Show resolved Hide resolved
docs/config.rst Show resolved Hide resolved
liquidprompt Outdated Show resolved Hide resolved
@nojhan
Copy link
Collaborator Author

nojhan commented Oct 24, 2022

Do I squash all commits into one ? (i.e. do you want to keep the extended error meaning setup?)

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.

Looks good now.

I missed that question the first time, sorry. I do not have much of an opinion. Clearly you intended for both bits to be part of the feature, so it would be fine in once commit. But if you want it as separate, I'm fine with that.

But no matter what, squash away the extra commits from the fixes and docs.

@nojhan
Copy link
Collaborator Author

nojhan commented Oct 24, 2022

The two commits were only there to make it easy to just have a single config, if you're OK with the "extended" config, I'll squash everything.

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.

Yeah, squash down to 1.

liquidprompt Outdated Show resolved Hide resolved
- Displays the (guessed) meaning of the last error code (feature request #729).
- Use a set of reasonably common error codes on POSIX systems
  (but is LP used out of a POSIX env?).
- Use the color than lp_error (namely LP_COLOR_ERR).
- Put in the default theme right after the error code.
- Code would be very much simpler/configurable with Bash 4's associative arrays,
  but here use a case switch with non-configurable error meanings.
- Use a separated LP_ENABLE_ERROR_MEANING.
- Adds LP_ENABLE_ERROR_MEANING_EXTENDED to allow for a larger set of error codes
  (let the user decide the level of guess for error meaning).
- doc: update prompt comparison with error meaning
@Rycieos Rycieos merged commit 28c2318 into master Oct 27, 2022
@Rycieos Rycieos deleted the feat+error-meaning branch October 27, 2022 15:35
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

2 participants