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.

Sign up for GitHub

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

Fall back to 'tac' [sic] when 'rev' is not available (see comments) #39

Closed
wants to merge 1 commit into from
Closed

Fall back to 'tac' [sic] when 'rev' is not available (see comments) #39

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion git-modified
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,9 @@ if [ -z "$commit" ]; then
fi
else
TAB=" " # literal tab char
git log -1 --name-status --pretty=format:"" "$commit" | cut -f2- | rev | cut -d"$TAB" -f1 | rev | make_relative | while read f; do
rev='rev'
type rev >/dev/null 2>&1 || rev='tac'
Copy link
Owner

Choose a reason for hiding this comment

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

Same request as in the other PR: let's not assume tac exists if rev does not, instead let's just do a test. In this case, you could make it a helper function:

reverse () {
  if type rev >/dev/null 2>&1; then
    rev
  elif type tac >/dev/null 2>&1; then
    tac
  else
    echo "Core utility 'rev' (nor 'tac') not found." >&2
    exit 2
  fi
}

And then call reverse in the pipe line, like

git log ... | reverse | cut -d"$TAB" -f1 | reverse | ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can totally do that. I just wasn't sure initially which side of the human-friendly-wrapper-functions-vs-lines-of-code question you would come down on, so I tried to keep the changes small.

Do you have strong opinions about line length? I would really like to break pipes over multiple lines at ~78 columns, but if it doesn't bother you, I won't let it bother me.

Copy link
Owner

Choose a reason for hiding this comment

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

I can totally do that. I just wasn't sure initially which side of the human-friendly-wrapper-functions-vs-lines-of-code question you would come down on, so I tried to keep the changes small.

Makes sense, it's a good default! In this case, the little extra helper function makes things easier to understand, so I'm ok with it :)

Do you have strong opinions about line length? I would really like to break pipes over multiple lines at ~78 columns, but if it doesn't bother you, I won't let it bother me.

That's fine in this case. This one was getting quite long in particular 👍

git log -1 --name-status --pretty=format:"" "$commit" | cut -f2- | $rev | cut -d"$TAB" -f1 | $rev | make_relative | while read f; do
if [ -f "$f" ]; then
echo "$f"
fi
Expand Down