Bash-it / bash-it

A community Bash framework.
MIT License
14.31k stars 2.29k forks source link

Fix call to liquidprompt function #2175

Closed DarrenBishop closed 3 weeks ago

DarrenBishop commented 2 years ago

Missing leading underscore in _lp_escape

Description

Motivation and Context

Without this - presumably - the code fails silently, never presenting the branch name in the PS1 prompt.

How Has This Been Tested?

Forgive me if there are tests for themes, but I could not find any; I personally tested this by cd'ing into and out of Git clone directories with and without the fix.

Screenshots (if appropriate):

Types of changes

Checklist:

davidpfarrell commented 2 years ago

Relevant information from Version 2.0 Upgrade Notes (dated Oct 6):

_lp_escape()

Renamed to __lp_escape. Return changed from stdout to $ret

Replace assignment statements like:

output="$(_lp_escape "$input")"

with:

local ret
__lp_escape "$input"
output=$ret

_lp_bzr_branch()

Return changed from stdout to $lp_vcs_branch

Recommended that _lp_vcs_branch is used instead.

Replace assignment statements like::

branch="$(_lp_bzr_branch)"

with::

local lp_vcs_branch
if _lp_bzr_branch; then
     branch=$lp_vcs_branch
fi
davidpfarrell commented 2 years ago

Best I can tell this meets the requirements for the recent 2.0 version, but if we publish it as-is, isn't it going to break the existing installations?

Not sure what the best way to deal with it is?

DarrenBishop commented 2 years ago

@davidpfarrell I am on the case

DarrenBishop commented 2 years ago

@davidpfarrell

Not sure what the best way to deal with it is?

It got a bit hairy at one point, but what I have done is:

Introduced some auxiliary functions

These functions are unset before the sourcing of Liquidprompt, to provide a clean slate if reloading the shell etc.

Updated _lp_git_branch to use the lp_vcs_branch return variable, but also print to stdout and (importantly) return 0 when in legacy mode.

I think a good practice going forward would be to pin the version of Liquidpromt to a tagged version, maybe via some env var, and print a warning/disclaimer if it's ever changed from the default.