Files that aren't in the commit are searched for commit comments

[SonOfLilit]

$ git commit
.git/hooks/prepare-commit-msg: line 43: conditional binary operator expected
.git/hooks/prepare-commit-msg: line 43: syntax error near `=~'
.git/hooks/prepare-commit-msg: line 43: `  if [[ "${IGNORED[*]}" =~ "${filename}" || "${IGNORED[*]}" =~ "${extension}" ]]; then'

$ cd ../commit-comments/
$ git log | head -n 1
commit a916600e8cc2b0cb0f6da2533d616da3ddca5f68

$ /bin/bash --version
GNU bash, version 3.1.20(4)-release (i686-pc-msys)
Copyright (C) 2005 Free Software Foundation, Inc.

Running

#!/bin/bash
ADDED_FILES=( $(git ls-files) )
echo $ADDED_FILES
echo '************'
for i in ${!ADDED_FILES[@]}; do
  file_path=${ADDED_FILES["$i"]}
  filename=$( basename "$file_path" )
  extension="${filename##*.}"
  echo $file_path
  echo $filename
  echo $extension
  if [[ "${IGNORED[*]}" =~ "${filename}" || "${IGNORED[*]}" =~ "${extension}" ]]; then
    # File should not be read - remove
    unset ADDED_FILES["$i"]
  fi
done

gives

$ . test.sh
.gitignore
************
sh.exe": ./test.sh: line 12: conditional binary operator expected
sh.exe": ./test.sh: line 12: syntax error near `=~'
sh.exe": ./test.sh: line 12: `  if [[ "${IGNORED[*]}" =~ "${filename}" || "${IGNORED[*]}" =~ "${extension}" ]]; then'

[SonOfLilit]

I tried to look into it.

Quickfix:

https://stackoverflow.com/questions/11206233/what-does-the-operator-do-in-shell-scripts

but .gitignore format is different from regex, and maybe it's better to use that?

https://stackoverflow.com/questions/2412450/git-pre-commit-hook-changed-added-files

but LarryH there points out that you'll pick up comments that are not up for being committed. That's bad because I love git add -i. He gives a solution, which is too big and architectural for me to PR without consulting with you first, and also I'm not sure I have time to do it, being as awful as I am in bash scripting.

In the meantime, I'll have to uninstall this :-(

[jryio]

Sorry this isn't working and I appreciate the research into the issue!

I've looked into the compatibility of the Regular Expression operator for Bash version 3.1 - Documentation. It explains that the Regular Expression operator is indeed present for Bash (so you're not missing the operator).

However one change which was made after Bash version 3.2 is that the right hand side of the operator should not be quoted Ex. [[ "string quoted" =~ [a-zA-z0-9]* ]]"

Can you test the following commands (just enter them directly into Bash terminal session) and reply with the output? 1.

if [[ "hello world" =~ ".*" ]]; then echo "it worked"; else echo "it failed"; fi

if [[ "hello world" =~ .* ]]; then echo "it worked"; else echo "it failed"; fi

That would help determine if that is your issue

---

I am unable to reproduce this bug using Bash version 3.1. Here are some of the tests I've done to replicate this.

• Created a virtual Linux machine running Ubuntu 14.04. Then installed Bash version 3.1 from https://ftp.gnu.org/gnu/bash/bash-3.1.tar.gz

root@test-server:~# bash --version
GNU bash, version 3.1.0(1)-release (x86_64-unknown-linux-gnu)
Copyright (C) 2005 Free Software Foundation, Inc.
root@test-server:~#

cd commit-comments

root@hi-predictor-api:~/commit-comments# ./test.sh
- Added new link parsing function
- Stored Hello World in string
- Inline commit comment
- Another inline commit comment
- final inline commit comment
- Added a new function to basic.cpp
- Added forEach function for NodeList elements

Also, I made another script with your modified contents and executed it: son-of-lilit.sh

root@test-server:~/commit-comments# ./son-of-lilit.sh
.ccignore
************
.ccignore
.ccignore
ccignore
LICENSE
LICENSE
LICENSE
README.md
README.md
md
post-commit
post-commit
post-commit
prepare-commit-msg
prepare-commit-msg
prepare-commit-msg
test-files/basic.cpp
basic.cpp
cpp
test-files/thing.js
thing.js
js
test.sh
test.sh
sh

---

Could you please add your Operating System & version & any other shells you have running?

Thank you!

[SonOfLilit]

$ if [[ "hello world" =~ ".*" ]]; then echo "it worked"; els
e echo "it failed"; fi
sh.exe": conditional binary operator expected
sh.exe": syntax error near `=~'
$ if [[ "hello world" =~ .* ]]; then echo "it worked"; else
echo "it failed"; fi
sh.exe": conditional binary operator expected
sh.exe": syntax error near `=~'

also:

[[ a =~ b ]]
sh.exe": conditional binary operator expected
sh.exe": syntax error near `=~'

I'm running Git Windows' bash on Windows 7. It has never shown deviance from standard before, but who knows.

$ git --version
git version 1.9.5.msysgit.0

I'll install 2.x and see if it helps.

However, I feel that the real issue I reported was this:

https://stackoverflow.com/questions/2412450/git-pre-commit-hook-changed-added-files

but LarryH there points out that you'll pick up comments that are not up for being committed. That's bad because I love git add -i. He gives a solution, which is too big and architectural for me to PR without consulting with you first, and also I'm not sure I have time to do it, being as awful as I am in bash scripting.

In the meantime, I'll have to uninstall this :-(

and it's far worse than some syntax error I have a slow workaround for, and even from .ccignore having different syntax than .gitignore's, don't you think?

[jryio]

Ah sorry for glossing over the second issue. Let me tackle those in order:

• Using git diff --cached --name-status --diff-filter=ACM is a great solution to cut down on the number of files to be searched. Additionally, as you've mentioned, if you're using git add -i and partially committing files, this again is a better method. Only looking at the staged files should have been my initial approach but I did not come across this command (so thank you for that :smile:). @SonOfLilit If you'd like to open a PR to add the git diff implementation I would be really happy to help you (maybe practice your Bash scripting? :rocket:). Regardless this is something I'll begin to work on right away.
• Having a different syntax is somewhat troublesome. I am considering reverting the .ccignore to be .gitignore syntax. Part of the reason I allowed filetypes like .cpp and .sh to be in .ccignore is due to several projects I have with thousands of files which I don't directly contribute to or maintain. In hindsight this feature is almost entirely useless, and keeping with the .gitignore syntax is better for usability. And as mentioned above, using the --diff-filter option removes the need for .ccignore entirely
• Just to double back for a second: If you're running Git Bash on Windows the =~ operator is actually unsupported! Perhaps this would solve the issue: http://stackoverflow.com/questions/15510083/syntax-error-operator-in-msysgit-bash

Edit: For portability I will no longer use the =~ operator and move to using echo $array | grep 'element' instead.

[SonOfLilit]

First the small things:

=~ indeed seems to be unsupported by old Git for Windows' bash. I installed a new one, it should solve that, but again, I think this implementation is buggy in ways that will affect me so I won't use it until that is solved.

An extension is ignored in .gitignore syntax by *.ext. Just one key more, definitely worth it. Also, the right time to make a breaking change, since the current syntax is yet undocumented.

You were looking at araqnid's response to that SO question. That's the less interesting one (it gives a better way to do more or less the same thing, but that's the wrong thing to do.)

Read LarryH's response. If you want to adopt his method, I will consider trying my hands at it, but it's not that simple (especially removing the comments that were staged but not others... I think we'll need to do it in the temporary copy, then generate a patch, then apply it to the working dir... but this smells like "there will be horrible complications".)

[jryio]

Pardon the reference, wrong issue number.

=~ is a perfectly valid operator in Bash, however I would also agree that replacing it with an echo $var | grep would be more appropriate and less prone to portability issues. Working on that now.

I did read LarryH's response, and saw his solution for creating a temp directory, but I think a simpler approach exists.

If we're solving the issue of only reading & removing comments from staged files (either Added, Modified, or Copied) then storing the result of git diff --name-status --diff-filter=ACM in the prepare-commit-msg hook and passing to the post-commit hook through a temporary file would suffice. Then the post-commit hook would be able to remove the comments only from those files which were staged.

Additionally, @hauleth just submitted Pull Request #3 which attempts to leave a clean repository after removing comments (no longer requiring a second commit once files are removed).

Would you say that tying your suggestion with @hauleth's addition would fully cover the issues?

[SonOfLilit]

The problem is that I can (and often do) "add" only part of a file. git add -i. Because of that, the temporary directory solution is really essential.

On Sat, Jan 16, 2016 at 8:35 AM Jacob Young [email protected] wrote:

Pardon the reference, wrong issue number.

=~ is a perfectly valid operator in Bash, however I would also agree that replacing it with an echo $var | grep would be more appropriate and less prone to portability issues. Working on that now.

I did read LarryH's response, and saw his solution for creating a temp directory, but I think a simpler approach exists.

If we're solving the issue of only reading & removing comments from staged files (either Added, Modified, or Copied) then storing the result of git diff --name-status --diff-filter=ACM in the prepare-commit-msg hook and passing to the post-commit hook through a temporary file would suffice. Then the post-commit hook would be able to remove the comments only from those files which were staged.

Additionally, @hauleth https://github.com/hauleth just submitted Pull Request #3 https://github.com/thebearjew/commit-comments/pull/3 which attempts to leave a clean repository after removing comments (no longer requiring a second commit once files are removed).

Would you say that tying your suggestion with @hauleth https://github.com/hauleth's addition would fully cover the issues?

— Reply to this email directly or view it on GitHub https://github.com/thebearjew/commit-comments/issues/1#issuecomment-172164582 .

[jryio]

Okay I clearly understand now.

Personally I do not use interactive Git adding using git add -i but I would assume you would be using the patch to select certain sections of a file diff to add, is that correct?

I'll try to incorporate this solution, however as you said, it might be a large architectural - but I will certainly attempt it

[SonOfLilit]

Woops, I meant git add -p, it's easier to use cousin. (I have it aliased to a for so long that I don't even remember the right syntax.)

Basically, with git you can commit an index that's completely unrelated to what's in your working copy at the time of committing. And many people do, quite often, me included.

On Sat, Jan 16, 2016 at 9:20 PM Jacob Young [email protected] wrote:

Okay I clearly understand now.

Personally I do not use interactive Git adding using git add -i but I would assume you would be using the patch to select certain sections of a file diff to add, is that correct?

I'll try to incorporate this solution, however as you said, it might be a large architectural - but I will certainly attempt it

— Reply to this email directly or view it on GitHub https://github.com/thebearjew/commit-comments/issues/1#issuecomment-172247920 .