git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH w/signoff] pre-push.sample: Remove unwanted `IFS=' '`.

From
Junio C Hamano <gitster@pobox.com>
Date
Dec 23, 2014, 02:08 UTC
Message-ID
<xmqq7fxj5d8k.fsf@gitster.dls.corp.google.com>
In-Reply-To
<xmqqlhm0botq.fsf@gitster.dls.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 17 quoted lines
> Jim Hill <gjthill@gmail.com> writes:
>
>> I call it unwanted because the default works fine with the actual
>> input and explicitly limiting whitespace this way breaks most command
>> substitution.
>
> OK.  I'd call that "unnecessary", not "unwanted", though.
>
> It becomes unwanted only when somebody cuts and pastes and changes
> what happens inside the body of the loop without thinking what IFS
> assignment is doing.
>
> Leaving it to the default is not wrong per-se, but I think it is
> better to justify this change as protecting cut-and-paste people,
> which is its primary benefit as far as I can see.
>
> Thanks for noticing.

FYI, here is what I queued for today's integration cycle (you should be able to find it in 'pu' branch).

-- >8 --
From: Jim Hill <gjthill@gmail.com>
Date: Sun, 21 Dec 2014 11:26:00 -0800
Subject: [PATCH] pre-push.sample: remove unnecessary and misleading IFS=' '

The sample hook explicitly sets IFS to SP and nothing else so that the "read" used in the per-ref while loop that iterates over "<localref> SP <localsha1> SP <remoteref> SP <remotesha>" records, where we know refs and sha1s will not have SPs, would split them correctly.

While this is not wrong per-se, it is not necessary; because we know these fields do not contain HT or LF, either, we can simply leave IFS the default.

This will also prevent those who cut and paste from this sample from getting bitten when they write things in the per-ref loop that need splitting with the default $IFS (e.g. use $(git rev-list ...) to produce one-record-per-line output).

Signed-off-by: Jim Hill <gjthill@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 templates/hooks--pre-push.sample | 1 -
 1 file changed, 1 deletion(-)
diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample
index 69e3c67..6187dbf 100755
--- a/templates/hooks--pre-push.sample
+++ b/templates/hooks--pre-push.sample
@@ -24,7 +24,6 @@ url="$2"
 
 z40=0000000000000000000000000000000000000000
 
-IFS=' '
 while read local_ref local_sha remote_ref remote_sha
 do
 	if [ "$local_sha" = $z40 ]
-- 
2.2.1-321-gd161b79
Previous: Junio C HamanoNext: Jim Hill
Message 6 of 7 in “pre-push.sample: Remove unwanted `IFS=' '`.”
  1. pre-push.sample: Remove unwanted `IFS=' '`.Jim Hill, Dec 21, 2014
  2. pre-push.sample: Remove unwanted `IFS=' '`.Jim Hill, Dec 21, 2014
  3. Junio C HamanoDec 21, 2014
  4. Jim HillDec 21, 2014
  5. Junio C HamanoDec 21, 2014
  6. Junio C HamanoDec 23, 2014
  7. pre-push.sample: remove unwanted `IFS=' '`.Jim Hill, Dec 21, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.