Re: [PATCH] difftool: handle changing symlinks in dir-diff mode

[Junio C Hamano <gitster@pobox.com>]

David Aguilar <davvid@gmail.com> writes:

> Reading the code again, the point of add_left_or_right()
> is to populate the worktree (done later in the loop) with
> the stuff we read from Git.  Thus, if we changed just this
> section to call get_symlink() then we should not even try
> to checkout any symlink entries in !use_wt_file(...)
> block where checkout_entry() / create_symlink_file() are called.
> 
> Since the symlinks2 hashmap already populates the worktree
> then that code should instead simply skip symlinks.

OK, that would simplify things, certainly.

> I'll take a stab at adding a get_symlink() helper, adjust
> the code so that add_left_or_right() is populated, and
> special-case the checkout_entry() code path to simply skip
> over null SHA1s.

Did you mean s/null SHA1s/S_ISLNK()s/?

> One minor thing I noticed is that I had to use "echo -n"
> for the stuff coming out of strbuf_readlink(), and
> plain "echo" for entries that come in via read_sha1_file()
> content passed to add_left_or_right().
>
> That suggests that maybe I should append a newline to the
> output from strbuf_readlink() so that it matches Git.
> Does that sound right?  Does Git store symlink entries
> with a terminating newline?

Do not append a newline.  Unless the pathname of the target you are
symlinking to ends with LF, readlink() won't end with LF, and the
stand-in file shouldn't, either.

By the way, avoid "echo -n"; use "printf '%s'" instead.

Thanks.