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

Re: [PATCH v3 05/10] diff-lib: accept option flags in run_diff_index()

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 17, 2020, 17:00 UTC
Message-ID
<xmqqh7rws5hm.fsf@gitster.c.googlers.com>
In-Reply-To
<496908ac10152edb9cbcfdb75c43b1d00e163e3c.1600328335.git.liu.denton@gmail.com>
Denton Liu <liu.denton@gmail.com> writes:
Show 12 quoted lines
> diff --git a/diff-lib.c b/diff-lib.c
> index 50521e2093..0a0e69113c 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -518,7 +518,7 @@ static int diff_cache(struct rev_info *revs,
>  	return unpack_trees(1, &t, &opts);
>  }
>  
> -int run_diff_index(struct rev_info *revs, int cached)
> +int run_diff_index(struct rev_info *revs, unsigned int option)
>  {
>  	struct object_array_entry *ent;
If we introduce
	int cached = !!(option & DIFF_INDEX_CACHED);

we do not have to touch the remainder of the function, and it makes it easier to read the place(s) where 'cached' is used. At that point, the fact that we were instructed by a bit in the option flag word is much much less important than we were instructed to compare the tree-ish with the index and not the working tree files.

The same technique is used with DIFF_RACY_IS_MODIFIED flag in run_diff_files().

Show 11 quoted lines
> diff --git a/diff.h b/diff.h
> index e0c0af6286..0cc874f2d5 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -585,7 +585,8 @@ const char *diff_aligned_abbrev(const struct object_id *sha1, int);
>  /* report racily-clean paths as modified */
>  #define DIFF_RACY_IS_MODIFIED 02
>  int run_diff_files(struct rev_info *revs, unsigned int option);
> -int run_diff_index(struct rev_info *revs, int cached);
> +#define DIFF_INDEX_CACHED 01
> +int run_diff_index(struct rev_info *revs, unsigned int option);

It is unclear from the above if the option word for run_diff_files() and run_diff_index() share the same bit assignment. After reading the series through to the end, I know they are independent set of bits and never shared, but I wonder if we can make it more obvious here.

Perhaps an extra blank line before this new #define is sufficient to make it clear?

Previous: Denton LiuNext: Denton Liu
Message 39 of 58 in “builtin/diff: learn --merge-base”
  1. 0/4 builtin/diff: learn --merge-baseDenton Liu, Sep 5, 2020
  2. 1/4 t4068: remove unnecessary >tmpDenton Liu, Sep 5, 2020
  3. 3/4 builtin/diff: parse --no-index using parse_options()Denton Liu, Sep 5, 2020
  4. 2/4 git-diff.txt: backtick quote command textDenton Liu, Sep 5, 2020
  5. 4/4 builtin/diff: learn --merge-baseDenton Liu, Sep 5, 2020
  6. Junio C HamanoSep 6, 2020
  7. 0/4 builtin/diff: learn --merge-baseDenton Liu, Sep 10, 2020
  8. 1/4 t4068: remove unnecessary >tmpDenton Liu, Sep 10, 2020
  9. 2/4 git-diff.txt: backtick quote command textDenton Liu, Sep 10, 2020
  10. 3/4 builtin/diff: parse --no-index using parse_options()Denton Liu, Sep 10, 2020
  11. Junio C HamanoSep 10, 2020
  12. Denton LiuSep 13, 2020
  13. Junio C HamanoSep 13, 2020
  14. 4/4 builtin/diff: learn --merge-baseDenton Liu, Sep 10, 2020
  15. 00/10 builtin/diff: learn --merge-baseDenton Liu, Sep 17, 2020
  16. 01/10 t4068: remove unnecessary >tmpDenton Liu, Sep 17, 2020
  17. 02/10 git-diff-index.txt: make --cached description a proper sentenceDenton Liu, Sep 17, 2020
  18. 10/10 contrib/completion: complete `git diff --merge-base`Denton Liu, Sep 17, 2020
  19. 09/10 builtin/diff-tree: learn --merge-baseDenton Liu, Sep 17, 2020
  20. Junio C HamanoSep 17, 2020
  21. Denton LiuSep 18, 2020
  22. Junio C HamanoSep 18, 2020
  23. Denton LiuSep 20, 2020
  24. Junio C HamanoSep 21, 2020
  25. Denton LiuSep 21, 2020
  26. Junio C HamanoSep 21, 2020
  27. Junio C HamanoSep 21, 2020
  28. Denton LiuSep 21, 2020
  29. Junio C HamanoSep 21, 2020
  30. Denton LiuSep 23, 2020
  31. Junio C HamanoSep 25, 2020
  32. Denton LiuSep 26, 2020
  33. 08/10 builtin/diff-index: learn --merge-baseDenton Liu, Sep 17, 2020
  34. Junio C HamanoSep 17, 2020
  35. Jeff KingSep 17, 2020
  36. Junio C HamanoSep 18, 2020
  37. Jeff KingSep 18, 2020
  38. 05/10 diff-lib: accept option flags in run_diff_index()Denton Liu, Sep 17, 2020
  39. Junio C HamanoSep 17, 2020
  40. 04/10 contrib/completion: extract common diff/difftool optionsDenton Liu, Sep 17, 2020
  41. 06/10 diff-lib: define diff_get_merge_base()Denton Liu, Sep 17, 2020
  42. Junio C HamanoSep 17, 2020
  43. Denton LiuSep 18, 2020
  44. Junio C HamanoSep 19, 2020
  45. 03/10 git-diff.txt: backtick quote command textDenton Liu, Sep 17, 2020
  46. 07/10 t4068: add --merge-base testsDenton Liu, Sep 17, 2020
  47. 00/10 builtin/diff: learn --merge-baseDenton Liu, Sep 20, 2020
  48. 01/10 t4068: remove unnecessary >tmpDenton Liu, Sep 20, 2020
  49. 02/10 git-diff-index.txt: make --cached description a proper sentenceDenton Liu, Sep 20, 2020
  50. 06/10 diff-lib: define diff_get_merge_base()Denton Liu, Sep 20, 2020
  51. 05/10 diff-lib: accept option flags in run_diff_index()Denton Liu, Sep 20, 2020
  52. 03/10 git-diff.txt: backtick quote command textDenton Liu, Sep 20, 2020
  53. 07/10 t4068: add --merge-base testsDenton Liu, Sep 20, 2020
  54. 08/10 builtin/diff-index: learn --merge-baseDenton Liu, Sep 20, 2020
  55. Martin ÅgrenSep 29, 2020
  56. 04/10 contrib/completion: extract common diff/difftool optionsDenton Liu, Sep 20, 2020
  57. 09/10 builtin/diff-tree: learn --merge-baseDenton Liu, Sep 20, 2020
  58. 10/10 contrib/completion: complete `git diff --merge-base`Denton Liu, Sep 20, 2020

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.