mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: ZheNing Hu <>
To: Junio C Hamano <>
Cc: ZheNing Hu via GitGitGadget <>,
	Git List <>, Denton Liu <>,
	David Aguilar <>,
	John Keeping <>,
	Ryan Zoeller <>
Subject: Re: [PATCH v3] difftool.c: learn a new way start from specified file
Date: Sun, 14 Feb 2021 15:53:44 +0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Junio C Hamano <> 于2021年2月11日周四 上午2:17写道:
> 胡哲宁 <> writes:
> > It has no effect on this new feature. I should put this modification
> > in an additional commit, right?
> Or you can just drop it.  It certainly is a distracting change to be
> part of this topic.
> > Yes, I want to know why being so cautious in git log?If the file name is
> > wrong, why can't I make it exit? :)
> Imagine a history where file1 and file2 are in the initial commit.
> The second commit adds file3 and modifies file2, and then the third
> commit modifies file1.  What would happen when you did this?
>   $ git log -p --rotate-to=file2
> For the commit at HEAD, the set of paths shown would be file1 and
> nothing else (because it is the only path that gets modified).  You
> cannot start showing from "file2".  Dying (and not showing HEAD~1
> and HEAD~2) is the last thing you want to do in this situation.  We
> do not even want to give a warning or an error, as it is totally
> expected that some commits do not touch a given path---it would be
> very unusual if a path is touched by every commit ;-).
Now I understand that in `log -p`, some commit do not have the file, and some
commit have the file. The best way to display the commit without the file
is to keep it as it is, and rotate the commit with the file. And in `difftool`
need to ensure the exist the specified file or give the more matching file
as the beginning(As you mentioned later).
> For "difftool --start-at=file2", the equation is different.  It does
> not traverse history where each commit may or may not modify file2,
> and when the user says s/he wants to resume from file2, file2 must
> be in the set of paths that have changes, or something is wrong (i.e.
> the user meant file3 but mistyped it as file2).
> > Awesome idea. In this way, `difftool --rotate-to=<file>` can call
> > `diff --rotate-to=<file>` , user can choose the starting file, and they can
> > also see previous files.
> So "difftool --start-at=<file>" can of use "diff --rotate-to=<file>"
> to implement the feature (after all, that is why I wrote it), but
> the error condition between the two are quite different.  And ...

> > After that, there was too little work I could do,do i just need to add
> > the following
> >  code in `diff_flush_patch_all_file_pairs`?
> > if (o->rotate_to && q->nr && strcmp(q->queue[0]->one->path, o->rotate_to) &&
> > strcmp(q->queue[0]->one->path, o->rotate_to)) {
> >     error(_("could not find start file name '%s'"), o->rotate_to);
> >         return;
> > }
> ... that is why an unconditional change like this in diff.c is not
> acceptable, as it would break other codepaths like "git log -p".  If
> we were to add an error there, it has to be very limited to exclude
> at least "log -p"---there may be other features that share the code
> that should not trigger an error for similar reasons.
> If diffcore-rotate chooses "missing rotate-to file makes it a no-op"
> as its semantics, and if "difftool --start-at" does not want to see
> a misspelt filename making it a no-op, then the latter needs to
> ensure that the name it got from the user is indeed in the set of
> paths that have changes before running "diff --rotate-to" to
> implement its "difftool --start-at" feature.
> The "missing rotate-to file in the diff_queue MUST NOT cause
> diffcore-rotate to error out" rule is probably unnegitiable, but
> there are other ways to make it easier to use, though.
> For example, we could change the rotate-to logic to mean "start at
> the given path, or if such a path does not exist in the diff_queue,
> then the first path that sorts after the given path is where we
> start".  That way, if the diff_queue has paths A B C D E and
> rotate-to gives C, then we rotate the output to C D E A B.  And if
> the diff_queue has A B D E and rotate-to gives C, then the output
> would become D E A B (instead of becoming a no-op).  Then, a mistyped
> filename may not do what the user wanted to do (after all, that is
> the definition of MIStyping), but it would do something noticeable
> by the user, which may be useful enough and at least would let the
> user notice the mistake.
In doing this, I think the processing methods of difftool and other diffs
are unified.I think this kind of processing is actually very easy,
just need to change
> if (!strcmp(rotate_to_filename, q->queue[i]->two->path))
> if (strcmp(rotate_to_filename, q->queue[i]->two->path) <= 0)
Of course, it might be better if there is an algorithm that can achieve
the highest degree of file name matching.

Now that `difftool --start-at` and `diff --rotate-to` are unified effects,
is "start-at" just an alias for "rotate-to"?
Or do I need to write like this?
> OPT_STRING(0, "start-with", &options->rotate-to, N_("<path>"),
>   N_("pass from difftool to diff, has the same effort as `rotate-to`")),

> > In addition, Do I need to do the documentation and tests related to
> > your `diff --rotate-to`?
> Once we know how we want "diff --rotate-to" to behave exactly, I can
> help that part further, if you want.  And then you can build on top.
> But we need to design exactly what the desired semantics would be
> before any of that.
> Thanks.

  parent reply	other threads:[~2021-02-14  7:54 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-07 15:19 [PATCH] learn a new way skip to save point 阿德烈 via GitGitGadget
2021-02-07 18:29 ` Junio C Hamano
2021-02-07 19:04 ` Junio C Hamano
2021-02-08  8:06   ` 胡哲宁
2021-02-08 17:02 ` [PATCH v2] learn a new way go back to last " ZheNing Hu via GitGitGadget
2021-02-08 20:13   ` Junio C Hamano
2021-02-08 22:15   ` David Aguilar
2021-02-08 23:34     ` Junio C Hamano
2021-02-09  6:19       ` 胡哲宁
2021-02-09  6:04     ` 胡哲宁
2021-02-09 15:30   ` [PATCH v3] difftool.c: learn a new way start from specified file ZheNing Hu via GitGitGadget
2021-02-09 18:17     ` Junio C Hamano
2021-02-10 17:00       ` 胡哲宁
2021-02-10 18:16         ` Junio C Hamano
2021-02-10 20:05           ` Junio C Hamano
2021-02-14  7:53           ` ZheNing Hu [this message]
2021-02-14 13:09     ` [PATCH v4] difftool.c: learn a new way start at " ZheNing Hu via GitGitGadget
2021-02-16  1:46       ` Junio C Hamano
2021-02-16 12:56       ` [PATCH v5 0/2] " ZheNing Hu via GitGitGadget
2021-02-16 12:56         ` [PATCH v5 1/2] diff: --{rotate,skip}-to=<path> Junio C Hamano via GitGitGadget
2021-02-16 12:56         ` [PATCH v5 2/2] difftool.c: learn a new way start at specified file ZheNing Hu via GitGitGadget
2021-02-17 10:31           ` Junio C Hamano
2021-02-17 16:18             ` ZheNing Hu
2021-02-16 18:45         ` [PATCH v5 0/2] " Junio C Hamano
2021-02-17  4:12           ` ZheNing Hu
2021-02-17 11:14             ` Denton Liu
2021-02-17 11:40               ` ZheNing Hu
2021-02-17 18:56                 ` Junio C Hamano
2021-02-18  5:20                   ` ZheNing Hu
2021-02-18 15:04                   ` ZheNing Hu
2021-02-18 19:11                     ` Junio C Hamano
2021-02-19 10:59                       ` ZheNing Hu
2021-02-25 11:08                   ` Johannes Schindelin
2021-02-25 19:01                     ` Junio C Hamano
2021-02-19 12:53         ` [PATCH v6] " ZheNing Hu via GitGitGadget
2021-02-22 15:11           ` ZheNing Hu
2021-02-22 21:40           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='' \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).