Thank you for reviewing this; I appreciate it! > > + content="foo" && > > + echo $content >not-empty && > > The quoting decision is backwards in these two lines. It is OK not > to quote when the right hand side literal is clearly a single word > without $IFS. On the other hand, it is a good practice to always > quote when using what is in a "$variable". Yes, that doesn't look right, I will make changes in v2. [...] > > > + touch empty && > > > > Use of "touch" gives a wrong impression that you care about the file > > timestamp; use something like ": >empty &&" instead when you care > > about the presence of the file and do not care about its timestamp. > > I just realized that this is even more important in this case not to > use "touch". > > The test that uses this file cares not just the presence, but it > deeply cares that its contents is empty. The thing it least cares > about is its timestamp. > > The purpose of using "touch" is to update the timestamp, to keep the > current contents if it exists, and to ensure it exists (as a side > effect), in the decreasing order of importance. Use of the command > here misleads the readers. Oops, you are right. That makes sense. Will update to ": >empty". [...] > > + git add -N empty not-empty && > > + git diff-files -p >actual && > > + hash_e=$(git hash-object empty) && > > + hash_n=$(git hash-object not-empty) && > > + cat >expect <<-EOF && > > + diff --git a/empty b/empty > > + new file mode 100644 > > + index 0000000..$(git rev-parse --short $hash_e) > > + diff --git a/not-empty b/not-empty > > + new file mode 100644 > > + index 0000000..$(git rev-parse --short $hash_n) > > + --- /dev/null > > + +++ b/not-empty > > + @@ -0,0 +1 @@ > > + +$content > > + EOF > > + test_cmp expect actual > > +' > > OK. Do we want to show what happens when "diff" and "diff --cached" > are run with these two "added but not quite added yet" paths to > contrast with this new case? I'm not sure if we want to repeat an older test. The test (which was renamed in this patch) in t2203-add-intent.sh: "diff/diff-cached shows ita as new/not-new files" is already doing that. Should the "diff" and "diff --cached" steps be appended here again? Thanks.
