Subject: Re: [PATCH v7 3/3] cat-file: add --follow-symlinks to --batch

From: Junio C Hamano <[email-masked]>
Date: Tue, 12 May 2015 20:57:27 -0700

[email-masked] writes:

> From: David Turner <[email-masked]>
>
> This wires the in-repo-symlink following code through to the cat-file
> builtin.  In the event of an out-of-repo link, cat-file will print
> the link in a new format.
>
> Signed-off-by: David Turner <[email-masked]>
> ---

Thanks.

> +--follow-symlinks::
> +	With --batch or --batch-check, follow symlinks inside the
> +	repository when requesting objects with extended SHA-1
> +...
> +	printed.
> +[normal]
> +	This option does not (currently) work correctly when an

Nice; didn't realize that "[normal]" trick would work, but why not.

> +	object in the index is specified (e.g. `:link` instead of
> +	`HEAD:link`) rather than one in the tree.
> +[normal]
> +	This option cannot be used unless `--batch` or `--batch-check`
> +	is used, because it would be impossible to distinguish between
> +	the output for an out-of-repo symlink, and the contents of a
> +	blob.
> +
> +For example, consider a git repository containing:

... but I am not sure about this paragraph.  Does it align and look
to be at the same level as the first paragraph that begins with
"With `--batch` or `--batch-check`" (asking because I do not have an
access to a good environment to check the resulting HTML easily
right now)?

> +	result = get_sha1_with_context(obj_name, flags, data->sha1, &ctx);
> +	if (result != FOUND) {
> +		switch(result) {

	switch (result) {

> +		case MISSING_OBJECT:
> +			printf("%s missing\n", obj_name);
> +			break;
> +		case DANGLING_SYMLINK:
> +			printf("dangling %"PRIuMAX"\n%s\n", strlen(obj_name),
> +			       obj_name);
> +			break;
> +		case SYMLINK_LOOP:
> +			printf("loop %"PRIuMAX"\n%s\n", strlen(obj_name),
> +			       obj_name);
> +			break;
> +		case NOT_DIR:
> +			printf("notdir %"PRIuMAX"\n%s\n", strlen(obj_name),
> +			       obj_name);
> +			break;

Don't all these lengths need to be cast to uintmax_t, like the
"symlink" one below?

By the way, I noticed that my compiler is so stupid that it does not
realize this "switch (result)" is inside "if (result != FOUND)" and
missing "case FOUND:" is perfectly fine.  The remainder of the code
I have been compiling with -Wswitch, but this one makes me to drop
it.

> +		}
> +		fflush(stdout);
> +		return 0;
> +	}
> +
> +	if (ctx.mode == 0) {
> +		printf("symlink %"PRIuMAX"\n%s\n",
> +		       (uintmax_t)ctx.symlink_path.len,
> +		       ctx.symlink_path.buf);
>  		fflush(stdout);
>  		return 0;