Added edit/remove functionality to git-ignore#1258
Conversation
spacewander
left a comment
spacewander
left a comment
There was a problem hiding this comment.
Let's update the docs, and completion script in https://github.com/tj/git-extras/tree/main/etc (if possible)
| } | ||
|
|
||
| _usage() { | ||
| # TODO |
There was a problem hiding this comment.
Let's either fulfill it or drop it
| level="none" | ||
| action="none" | ||
| declare -a args | ||
| while [ "$1" != "" ]; do |
There was a problem hiding this comment.
It breaks documented empty-string patterns. while [ "$1" != "" ] stops parsing as soon as a pattern is "", so existing examples like git ignore --local "" "# Temporary files" no longer add anything after the empty argument. Use while [ $# -gt 0 ] instead.
| local file="${2/#~/$HOME}" | ||
| if [ -f "$file" ]; then | ||
| echo "Removing patterns from $1 gitignore ($2)..." | ||
| while read -r line; do |
There was a problem hiding this comment.
It can corrupt preserved lines during remove. read without IFS= trims leading/trailing whitespace, echo "$line" mishandles values like -n, and the loop skips a final line without a trailing newline. Use while IFS= read -r line || [ -n "$line" ]; do and printf '%s\n' "$line".
There was a problem hiding this comment.
While thinking about these issues found that perl has \E \Q escapes. These could work too (something like perl -pe "s/\Q${pattern1}\E\n?//; s/\Q${pattern2}\E\n?//; ..."). Which of the approaches do you think is better?
There was a problem hiding this comment.
I wonder if we have to add perl in https://github.com/tj/git-extras/blob/main/Installation.md#dependencies. Could it be solved with awk or sed?
| edit_file() { | ||
| local file="${2/#~/$HOME}" | ||
| if [ -f "$file" ]; then | ||
| echo "Editing $1 gitignore ($2)..." && $(git var GIT_EDITOR) "$file" |
There was a problem hiding this comment.
$(git var GIT_EDITOR) "$file" loses shell quoting for editor values like vim -c 'set ft=gitignore', and echo "Done." still runs even if the editor exits non-zero.
There was a problem hiding this comment.
Changed the line to use eval - it seems to fix this issue.
| tmpfile="$(mktemp -q "${TMPDIR:-/tmp}"/git-extras-ignore.XXXXXX 2>/dev/null)" | ||
| else | ||
| cd_to_git_root --warn &>/dev/null | ||
| tmpfile="$(mktemp -q "$GIT_DIR"/git-extras-ignore.XXXXXX 2>/dev/null)" |
There was a problem hiding this comment.
Why don't we always put the temp file in $TMPDIR?
There was a problem hiding this comment.
Wanted it to work the same way git does and it writes its tmp files in .git. Changed to always use global tmp directory.
| echo "Removed $line pattern." | ||
| fi | ||
| done < "$file" | ||
| mv "$tmpfile" "$file" # tmp file created in get_tmp_file 'deleted' |
There was a problem hiding this comment.
If .gitignore or the global ignore file is a symlink, mv "$tmpfile" "$file" replaces the symlink instead of updating its target; it can also lose file metadata. Can we update it in place?
…ndling, tmp file location, symlinked gitignore handling).
2 participants
PR for issue #1257
These commits introduce flags -r/--remove and -e/--edit to git-ignore to allow removing patterns the same way they can be added and for opening ignore files in a text editor. Some other minor changes like '--' flag-argument separator introduced. New version is mostly backwards-compatible with old one.
Some quirks/things to consider:
Submitting this as a draft, and if everything's OK/after changes are made will modify documentation accordingly (same for output with -h/--help flags).