Giter Club home page Giter Club logo

Comments (9)

SonOfLilit avatar SonOfLilit commented on June 1, 2024

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 :-(

from commit-comments.

jryio avatar jryio commented on June 1, 2024

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.

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!

from commit-comments.

SonOfLilit avatar SonOfLilit commented on June 1, 2024
$ 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?

from commit-comments.

jryio avatar jryio commented on June 1, 2024

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 😄).
    @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? 🚀). 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.

from commit-comments.

SonOfLilit avatar SonOfLilit commented on June 1, 2024

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".)

from commit-comments.

jryio avatar jryio commented on June 1, 2024

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?

from commit-comments.

SonOfLilit avatar SonOfLilit commented on June 1, 2024

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 #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
#1 (comment)
.

from commit-comments.

jryio avatar jryio commented on June 1, 2024

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

from commit-comments.

SonOfLilit avatar SonOfLilit commented on June 1, 2024

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
#1 (comment)
.

from commit-comments.

Related Issues (9)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.