Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 29 May 2013 08:00:51 +0200
From: magnum <>
Subject: Re: Re: pre-commit hooks (was: Unify internal form of mscash2 hashes?)

On 29 May, 2013, at 5:43 , Sayantan Datta <> wrote:
> On Wed, May 29, 2013 at 4:56 AM, magnum <> wrote:
>> Which of them all? None of them works as described, I see different problems with all of them. Even after fixing that, there would be one SERIOUS problem left. Consider the following:
>> 1. You made lots of edits
>> 2. You selectively stage SOME of the edits (eg. only some changes to a certain file because you want to stage other changes to a different commit)
>> 3. Commit.
>> Every single one of the example hooks described will re-add ALL of the file after fixing trailing spaces, as opposed to the selectively staged parts. This is absolutely not the wanted behavior, especially if this happens silently. This is a total blocker - forget about all of them.
>> A 100% working, safe, hook script is very very tricky to come up with, especially if you want it compatible with different flavors of sed et al. It's a far better solution IMHO to teach your editor to do the right thing.
> I was partially wrong about the hook not working on multiple files but still the hook is far from perfect. I didn't edit much and used the 2nd one which had a minor error.  I am attaching the script.  The script works only if I follow certain steps:
> 1. Modify the files and deliberately add trailing white space to any line. Hook doesn't detect any white space in a file unless an extra white space is added to the file before staging them. 
> 2. Do git add -A 
> 3. Do git commit (pre-commit hook should eliminate all trailing white spaces from all modified files)
> 4. Again do git add -A (stage the files with no trailing white spaces)
> 5. Do git commit --no-verify (byepass the hook)   

That sounds overly complicated to me and it doesn't even drop all trailing white space. To do that, change 's/ *$//' to 's/[[:space::]]+$//'.


Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.