git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: Git List <git@vger.kernel.org>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 3/5] use SWAP macro
Date: Mon, 30 Jan 2017 14:22:10 -0800	[thread overview]
Message-ID: <xmqqr33ktch9.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <187c2b39-40cf-7e07-b489-d40cdf5f9145@web.de> ("René Scharfe"'s message of "Sat, 28 Jan 2017 22:40:58 +0100")

René Scharfe <l.s.r@web.de> writes:

>  		if (tree2->flags & UNINTERESTING) {
> -			struct object *tmp = tree2;
> -			tree2 = tree1;
> -			tree1 = tmp;
> +			SWAP(tree2, tree1);

A human would have written this SWAP(tree1, tree2).

Not that I think such a manual fix-up should be made in _this_
patch, which may end up mixing mechanical conversion (which we may
want to keep reproducible) and hand tweaks.  But this swapped swap
reads somewhat silly.

> diff --git a/diff-no-index.c b/diff-no-index.c
> index f420786039..1ae09894d7 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -186,9 +186,8 @@ static int queue_diff(struct diff_options *o,
>  
>  		if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
>  			unsigned tmp;
> -			const char *tmp_c;
>  			tmp = mode1; mode1 = mode2; mode2 = tmp;
> -			tmp_c = name1; name1 = name2; name2 = tmp_c;
> +			SWAP(name1, name2);

Curious that mode swapping is left for a later iteration.

> diff --git a/diff.c b/diff.c
> index f08cd8e033..9de1ba264f 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -5118,13 +5118,11 @@ void diff_change(struct diff_options *options,
>  
>  	if (DIFF_OPT_TST(options, REVERSE_DIFF)) {
>  		unsigned tmp;
> -		const unsigned char *tmp_c;
> -		tmp = old_mode; old_mode = new_mode; new_mode = tmp;
> -		tmp_c = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_c;
> +		SWAP(old_mode, new_mode);
> +		SWAP(old_sha1, new_sha1);
>  		tmp = old_sha1_valid; old_sha1_valid = new_sha1_valid;
>  			new_sha1_valid = tmp;

So is this one.

> diff --git a/merge-recursive.c b/merge-recursive.c
> ...
> -			tmp = ren2;
> -			ren2 = ren1;
> -			ren1 = tmp;
> +			SWAP(ren2, ren1);

A human would have written this SWAP(ren1, ren2).


  parent reply	other threads:[~2017-01-30 22:22 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-28 21:13 [PATCH 0/5] introduce SWAP macro René Scharfe
2017-01-28 21:38 ` [PATCH 1/5] add " René Scharfe
2017-01-30 15:39   ` Johannes Schindelin
2017-01-30 16:48     ` René Scharfe
2017-01-30 20:48       ` Johannes Schindelin
2017-01-30 21:46         ` René Scharfe
2017-01-31 12:13           ` Johannes Schindelin
2017-01-31 21:02             ` René Scharfe
2017-02-01  0:44               ` Ramsay Jones
2017-02-01 11:39               ` Johannes Schindelin
2017-01-30 16:01   ` Johannes Schindelin
2017-01-30 16:59     ` René Scharfe
2017-01-30 18:41     ` Johannes Sixt
2017-01-30 21:03       ` Johannes Schindelin
2017-01-30 22:09         ` René Scharfe
2017-01-30 22:21           ` Brandon Williams
2017-01-31 21:03             ` René Scharfe
2017-01-31 21:35               ` Jeff King
2017-01-31 22:29                 ` Junio C Hamano
2017-01-31 22:36                   ` Jeff King
2017-02-01 11:28                 ` Johannes Schindelin
2017-02-01 11:47                   ` Jeff King
2017-02-01 18:06                     ` René Scharfe
2017-02-01 18:33                       ` Junio C Hamano
2017-02-07 22:04                         ` René Scharfe
2017-02-07 22:30                           ` Junio C Hamano
2017-02-08 15:14                             ` Johannes Schindelin
2017-01-31 12:03           ` Johannes Schindelin
2017-04-24 11:29   ` Jeff King
2017-04-24 11:49     ` Jeff King
2017-04-24 13:13       ` Duy Nguyen
2017-04-28 17:04     ` René Scharfe
2017-04-28 21:49       ` Jeff King
2017-04-29 18:16         ` René Scharfe
2017-04-30  3:11           ` Jeff King
2017-05-02  5:29             ` René Scharfe
2017-01-28 21:40 ` [PATCH 2/5] apply: use " René Scharfe
2017-01-28 21:40 ` [PATCH 3/5] " René Scharfe
2017-01-30 16:03   ` Johannes Schindelin
2017-01-30 17:18     ` René Scharfe
2017-01-30 22:22   ` Junio C Hamano [this message]
2017-01-31 21:02     ` René Scharfe
2017-01-28 21:41 ` [PATCH 4/5] diff: " René Scharfe
2017-01-30 16:04   ` Johannes Schindelin
2017-01-30 17:26     ` René Scharfe
2017-01-30 22:22   ` Junio C Hamano
2017-01-28 21:42 ` [PATCH 5/5] graph: " René Scharfe
2017-01-30 16:16   ` Johannes Schindelin
2017-01-30 17:41     ` René Scharfe
2017-01-30 23:20 ` [PATCH 0/5] introduce " 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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqqr33ktch9.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=l.s.r@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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

	https://80x24.org/mirrors/git.git

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).