( )⚙ D796 scm-prompt: standardize testing style and formatting

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D796

scm-prompt: standardize testing style and formatting
ClosedPublic

Authored by ryanmce on Sep 22 2017, 10:37 AM.
Tags
None
Subscribers
mqian, stash

Details

Reviewers
stash
Group Reviewers
Restricted Project
Commits
rFBHGX7d5162acb093: scm-prompt: standardize testing style and formatting
Test Plan

rt

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ryanmce created this revision.Sep 22 2017, 10:37 AM
stash accepted this revision.Sep 22 2017, 10:45 AM
stash added a subscriber: stash.
stash added inline comments.
scripts/scm-prompt.sh
64

Why do we prefer builtin here? Can you also add it in the big comment above, that we should use "builtin" for echo and "command" for everything else

101

Same question as in previous diff - are there any practical implications, or it's just a style thing?

This revision is now accepted and ready to land.Sep 22 2017, 10:45 AM
Closed by commit rFBHGX7d5162acb093: scm-prompt: standardize testing style and formatting (authored by ryanmce). · Explain WhySep 22 2017, 11:37 AM
This revision was automatically updated to reflect the committed changes.
ryanmce added inline comments.Sep 22 2017, 11:57 AM
scripts/scm-prompt.sh
64

Whoops I missed this comment. I'll add a follow-up.

I changed to builtin because it works better in zsh -- in fact command cd doesn't work in zsh at all for my purposes, so I just tried to be consistent and use builtin for things I knew were builtin to the shells we support.

101

It's important if there are spaces or glob characters like *

mqian added a subscriber: mqian.Sep 26 2017, 5:19 PM
mqian added inline comments.
scripts/scm-prompt.sh
193

Adding the outer double quotes without removing the inner quotes causes literal quotes to appear in the prompt.

ryanmce added a comment.Sep 26 2017, 11:10 PM

I'm not sure how my testing missed this; occur a task last night to fix it and will get it in today.

ryanmce added inline comments.Sep 27 2017, 5:12 AM
scripts/scm-prompt.sh
193

Fixed in D823

Revision Contents
Changeset List

Diff 2028

scripts/scm-prompt.sh

# #
# Notes to developers: # Notes to developers:
# #
# * Aliases can screw up the default commands; use "command" to prevent this # * Aliases can screw up the default commands; use "command" to prevent this
# #
# ========================================================================= # =========================================================================
# #
_find_most_relevant() _find_most_relevant() {
{
# We don't want to output all remote bookmarks because there can be many # We don't want to output all remote bookmarks because there can be many
# of them. This function finds the most relevant remote bookmark using this # of them. This function finds the most relevant remote bookmark using this
# algorithm: # algorithm:
# 1. If 'master' or '@' bookmark is available then output it # 1. If 'master' or '@' bookmark is available then output it
# 2. Sort remote bookmarks and output the first in reverse sorted order ( # 2. Sort remote bookmarks and output the first in reverse sorted order (
# it's a heuristic that tries to find the newest bookmark. It will work well # it's a heuristic that tries to find the newest bookmark. It will work well
# with bookmarks like 'release20160926' and 'release20161010'). # with bookmarks like 'release20160926' and 'release20161010').
relevantbook=$(command grep -m1 -E -o "^[^/]+/(master|@)$" <<< "$1") relevantbook="$(command grep -m1 -E -o "^[^/]+/(master|@)$" <<< "$1")"
if [[ -n $relevantbook ]]; then if [[ -n $relevantbook ]]; then
command echo $relevantbook builtin echo $relevantbook
stashUnsubmitted
Not Done

Why do we prefer builtin here? Can you also add it in the big comment above, that we should use "builtin" for echo and "command" for everything else

stash: Why do we prefer builtin here? Can you also add it in the big comment above, that we should use…
ryanmceAuthorUnsubmitted
Not Done

Whoops I missed this comment. I'll add a follow-up.

I changed to builtin because it works better in zsh -- in fact command cd doesn't work in zsh at all for my purposes, so I just tried to be consistent and use builtin for things I knew were builtin to the shells we support.

ryanmce: Whoops I missed this comment. I'll add a follow-up. I changed to builtin because it works…
return 0 return 0
fi fi
command echo "$(command sort -r <<< "$1" | command head -n 1)" builtin echo "$(command sort -r <<< "$1" | command head -n 1)"
} }
_hg_prompt() { _hg_prompt() {
local hg br extra local hg br extra
hg="$1" hg="$1"
if [[ -f "$hg/bisect.state" ]]; then if [[ -f "$hg/bisect.state" ]]; then
extra="|BISECT" extra="|BISECT"
elif [[ -f "$hg/histedit-state" ]]; then elif [[ -f "$hg/histedit-state" ]]; then
extra="|HISTEDIT" extra="|HISTEDIT"
elif [[ -f "$hg/graftstate" ]]; then elif [[ -f "$hg/graftstate" ]]; then
extra="|GRAFT" extra="|GRAFT"
elif [[ -f "$hg/unshelverebasestate" ]]; then elif [[ -f "$hg/unshelverebasestate" ]]; then
extra="|UNSHELVE" extra="|UNSHELVE"
elif [[ -f "$hg/rebasestate" ]]; then elif [[ -f "$hg/rebasestate" ]]; then
extra="|REBASE" extra="|REBASE"
elif [[ -d "$hg/merge" ]]; then elif [[ -d "$hg/merge" ]]; then
extra="|MERGE" extra="|MERGE"
elif [[ -L "$hg/store/lock" ]]; then elif [[ -L "$hg/store/lock" ]]; then
extra="|STORE-LOCKED" extra="|STORE-LOCKED"
elif [[ -L "$hg/wlock" ]]; then elif [[ -L "$hg/wlock" ]]; then
extra="|WDIR-LOCKED" extra="|WDIR-LOCKED"
fi fi
local dirstate=$( \ local dirstate="$( \
(test -f "$hg/dirstate" && \ ( [[ -f "$hg/dirstate" ]] && \
command hexdump -vn 20 -e '1/1 "%02x"' "$hg/dirstate") || \ command hexdump -vn 20 -e '1/1 "%02x"' "$hg/dirstate") || \
(test -f "$hg/../.eden/client/SNAPSHOT" && \ ( [[ -f "$hg/../.eden/client/SNAPSHOT" ]] && \
command hexdump -s 8 -vn 20 -e '1/1 "%02x"' \ command hexdump -s 8 -vn 20 -e '1/1 "%02x"' \
"$hg/../.eden/client/SNAPSHOT") || \ "$hg/../.eden/client/SNAPSHOT") || \
command echo "empty") builtin echo "empty")"
local active="$hg/bookmarks.current" local active="$hg/bookmarks.current"
if [[ -f "$active" ]]; then if [[ -f "$active" ]]; then
br=$(command cat "$active") br="$(command cat "$active")"
stashUnsubmitted
Not Done

Same question as in previous diff - are there any practical implications, or it's just a style thing?

stash: Same question as in previous diff - are there any practical implications, or it's just a style…
ryanmceAuthorUnsubmitted
Not Done

It's important if there are spaces or glob characters like *

ryanmce: It's important if there are spaces or glob characters like *
# check to see if active bookmark needs update (eg, moved after pull) # check to see if active bookmark needs update (eg, moved after pull)
local marks="$hg/bookmarks" local marks="$hg/bookmarks"
if [[ -f "$hg/sharedpath" && -f "$hg/shared" ]] && if [[ -f "$hg/sharedpath" && -f "$hg/shared" ]] &&
command grep -q '^bookmarks$' "$hg/shared"; then command grep -q '^bookmarks$' "$hg/shared"; then
marks="$(command cat $hg/sharedpath)/bookmarks" marks="$(command cat $hg/sharedpath)/bookmarks"
fi fi
if [[ -z "$extra" ]] && [[ -f "$marks" ]]; then if [[ -z "$extra" ]] && [[ -f "$marks" ]]; then
local markstate=$(command grep " $br$" "$marks" | \ local markstate="$(command grep " $br$" "$marks" | \
command cut -f 1 -d ' ') command cut -f 1 -d ' ')"
if [[ $markstate != "$dirstate" ]]; then if [[ $markstate != "$dirstate" ]]; then
extra="|UPDATE_NEEDED" extra="|UPDATE_NEEDED"
fi fi
fi fi
else else
br=$(command echo "$dirstate" | command cut -c 1-7) br="$(builtin echo "$dirstate" | command cut -c 1-7)"
fi fi
local remote="$hg/remotenames" local remote="$hg/remotenames"
if [[ -f "$remote" ]]; then if [[ -f "$remote" ]]; then
local allremotemarks="$(command grep "^$dirstate bookmarks" "$remote" | \ local allremotemarks="$(command grep "^$dirstate bookmarks" "$remote" | \
command cut -f 3 -d ' ')" command cut -f 3 -d ' ')"
if [[ -n "$allremotemarks" ]]; then if [[ -n "$allremotemarks" ]]; then
local remotemark="$(_find_most_relevant "$allremotemarks")" local remotemark="$(_find_most_relevant "$allremotemarks")"
if [[ -n "$remotemark" ]]; then if [[ -n "$remotemark" ]]; then
br="$br|$remotemark" br="$br|$remotemark"
if [[ "$remotemark" != "$allremotemarks" ]]; then if [[ "$remotemark" != "$allremotemarks" ]]; then
# if there is more than one, let the user know with an elipsis # if there is more than one, let the user know with an elipsis
br="${br}..." br="${br}..."
fi fi
fi fi
fi fi
fi fi
local branch local branch
if [[ -f "$hg/branch" ]]; then if [[ -f "$hg/branch" ]]; then
branch=$(command cat "$hg/branch") branch="$(command cat "$hg/branch")"
if [[ $branch != "default" ]]; then if [[ "$branch" != "default" ]]; then
br="$br|$branch" br="$br|$branch"
fi fi
fi fi
br="$br$extra" br="$br$extra"
printf "%s" "$br" printf "%s" "$br"
} }
_git_prompt() { _git_prompt() {
local git br local git br
git="$1" git="$1"
if test -f "$git/HEAD" ; then if [[ -f "$git/HEAD" ]]; then
read br < "$git/HEAD" read br < "$git/HEAD"
case $br in case $br in
ref:\ refs/heads/*) br=${br#ref: refs/heads/} ;; ref:\ refs/heads/*) br=${br#ref: refs/heads/} ;;
*) br=$(command echo "$br" | command cut -c 1-7) ;; *) br="$(builtin echo "$br" | command cut -c 1-7)" ;;
esac esac
if [[ -f "$git/rebase-merge/interactive" ]]; then if [[ -f "$git/rebase-merge/interactive" ]]; then
b="$(command cat "$git/rebase-merge/head-name")" b="$(command cat "$git/rebase-merge/head-name")"
b=${b#refs/heads/} b="${b#refs/heads/}"
br="$br|REBASE-i|$b" br="$br|REBASE-i|$b"
elif [[ -d "$git/rebase-merge" ]]; then elif [[ -d "$git/rebase-merge" ]]; then
b="$(command cat "$git/rebase-merge/head-name")" b="$(command cat "$git/rebase-merge/head-name")"
b=${b#refs/heads/} b="${b#refs/heads/}"
br="$br|REBASE-m|$b" br="$br|REBASE-m|$b"
else else
if [[ -d "$git/rebase-apply" ]]; then if [[ -d "$git/rebase-apply" ]]; then
if [[ -f "$git/rebase-apply/rebasing" ]]; then if [[ -f "$git/rebase-apply/rebasing" ]]; then
b="$(command cat "$git/rebase-apply/head-name")" b="$(command cat "$git/rebase-apply/head-name")"
b=${b#refs/heads/} b="${b#refs/heads/}"
br="$br|REBASE|$b" br="$br|REBASE|$b"
elif [[ -f "$git/rebase-apply/applying" ]]; then elif [[ -f "$git/rebase-apply/applying" ]]; then
br="$br|AM" br="$br|AM"
else else
br="$br|AM/REBASE" br="$br|AM/REBASE"
fi fi
elif [[ -f "$git/CHERRY_PICK_HEAD" ]]; then elif [[ -f "$git/CHERRY_PICK_HEAD" ]]; then
br="$br|CHERRY-PICKING" br="$br|CHERRY-PICKING"
elif [[ -f "$git/REVERT_HEAD" ]]; then elif [[ -f "$git/REVERT_HEAD" ]]; then
br="$br|REVERTING" br="$br|REVERTING"
elif [[ -f "$git/MERGE_HEAD" ]]; then elif [[ -f "$git/MERGE_HEAD" ]]; then
br="$br|MERGE" br="$br|MERGE"
elif [[ -f "$git/BISECT_LOG" ]]; then elif [[ -f "$git/BISECT_LOG" ]]; then
br="$br|BISECT" br="$br|BISECT"
fi fi
fi fi
fi fi
printf "%s" "$br" printf "%s" "$br"
} }
_scm_prompt() _scm_prompt() {
{
local dir fmt br local dir fmt br
# Default to be compatable with __git_ps1. In particular: # Default to be compatable with __git_ps1. In particular:
# - provide a space for the user so that they don't have to have # - provide a space for the user so that they don't have to have
# random extra spaces in their prompt when not in a repo # random extra spaces in their prompt when not in a repo
# - provide parens so it's differentiated from other crap in their prompt # - provide parens so it's differentiated from other crap in their prompt
fmt=${1:-' (%s)'} fmt="${1:-' (%s)'}"
mqianUnsubmitted
Not Done

Adding the outer double quotes without removing the inner quotes causes literal quotes to appear in the prompt.

mqian: Adding the outer double quotes without removing the inner quotes causes literal quotes to…
ryanmceAuthorUnsubmitted
Not Done

Fixed in D823

ryanmce: Fixed in D823
# find out if we're in a git or hg repo by looking for the control dir # find out if we're in a git or hg repo by looking for the control dir
dir=$PWD dir="$PWD"
while : ; do while : ; do
if test -d "$dir/.git" ; then if [[ -d "$dir/.git" ]]; then
br=$(_git_prompt "$dir/.git") br="$(_git_prompt "$dir/.git")"
break break
elif test -d "$dir/.hg" ; then elif [[ -d "$dir/.hg" ]]; then
br=$(_hg_prompt "$dir/.hg") br="$(_hg_prompt "$dir/.hg")"
break break
fi fi
test "$dir" = "/" && break [[ "$dir" = "/" ]] && break
# portable "realpath" equivalent # portable "realpath" equivalent
dir=$(cd -P "$dir/.." && command echo "$PWD") dir="$(builtin cd -P "$dir/.." && builtin echo "$PWD")"
done done
if [[ -n "$br" ]]; then if [[ -n "$br" ]]; then
printf "$fmt" "$br" builtin printf "$fmt" "$br"
fi fi
} }
# #
# Backwards-compatibility layer for odl scm-prompt script # Backwards-compatibility layer for odl scm-prompt script
# #
# Older versions of this file at Facebook used a longer function name. # Older versions of this file at Facebook used a longer function name.
# These versions also included support for an environmental directive # These versions also included support for an environmental directive