Thread (2 messages) flat view 2 messages, 2 authors, 2020-06-19

Re: [PATCH] diff-files: treat "i-t-a" files as "not-in-index"

From: Srinidhi Kaushik <hidden>
Date: 2020-06-19 09:34:16

Thank you for reviewing this; I appreciate it!
quoted
+     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.

[...]
quoted
quoted
+  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".

[...]
quoted
+     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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help