Re: [PATCH v6 1/3] ls_files.c: bugfix for --deleted and --modified

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"ZheNing Hu via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:

> From: ZheNing Hu <adlternative@xxxxxxxxx>
>
> This situation may occur in the original code: lstat() failed
> but we use `&st` to feed ie_modified() later.
>
> Therefore, we can directly execute show_ce without the judgment of
> ie_modified() when lstat() has failed.
>
> Signed-off-by: ZheNing Hu <adlternative@xxxxxxxxx>
> ---
>  builtin/ls-files.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)

Looks good.  I think we are finished with this part, except for one
nit.

> +			if (stat_err && show_deleted)
>  				show_ce(repo, dir, ce, fullname.buf, tag_removed);
> -			if (show_modified && ie_modified(repo->index, ce, &st, 0))
> -				show_ce(repo, dir, ce, fullname.buf, tag_modified);
> +			if (show_modified &&
> +				(stat_err || ie_modified(repo->index, ce, &st, 0)))
> +					show_ce(repo, dir, ce, fullname.buf, tag_modified);
>  		}

The last line is misindented by having one leading horizontal tab
too many.  show_ce() for modified files and show_ce() for deleted
files are done independently under different conditions and stand as
equals, so the beginning of them should align to show that.

Perhaps format the last three lines more like so:

			if (show_modified &&
			    (stat_err || ie_modified(repo->index, ce, &st, 0)))
				show_ce(repo, dir, ce, fullname.buf, tag_modified);

Again this would cascade throughout the sreies, but let's see if
there are other things we may want to change in the rest of the
series first.  Otherwise, instead of having you rebase, I probably
have time to tweak the series on my end while queuing.

Thanks.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux