All of
 help / color / mirror / Atom feed
From: Junio C Hamano <>
	Matheus Tavares <>
Subject: Re: [PATCH v2] Make ident dynamic, not just a hardcoded value of "$Id".
Date: Thu, 02 Sep 2021 12:04:11 -0700	[thread overview]
Message-ID: <xmqqmtoukdec.fsf@gitster.g> (raw)
In-Reply-To: <xmqqr1e77q01.fsf@gitster.g> (Junio C. Hamano's message of "Wed, 01 Sep 2021 17:58:06 -0700")

Junio C Hamano <> writes:

> If we are not restricting the characters that can be used in the
> custom ID placeholder, we probably should have tests that use
> allowed but unusual characters.

There is a test in t/ that ensures that we
recognise expanded and unexpanded keyword strings using various
lines that has placeholder-lookalike on it, which starts like this:

    # If an expanded ident ever gets into the repository, we want to make sure that
    # it is collapsed before being expanded again on checkout
    test_expect_success expanded_in_repo '
                    echo "File with expanded keywords"
                    echo "\$Id\$"
                    echo "\$Id:\$"
                    echo "\$Id: 0000000000000000000000000000000000000000 \$"
                    echo "\$Id: NoSpaceAtEnd\$"
                    echo "\$Id:NoSpaceAtFront \$"
                    echo "\$Id:NoSpaceAtEitherEnd\$"
                    echo "\$Id: NoTerminatingSymbol"
                    echo "\$Id: Foreign Commit With Spaces \$"
            } >expanded-keywords.0 &&

It would be good to have a copy of this test with the placeholder
customized to something unusual like " Id:", "Id: $ $", ":", "$ $"
etc. to make sure we can parse out the keyword correctly without
imposing the letters that can be used in the placeholder.

    Side note.  I of course do not literally mean to "copy" and make
    the number of tests explode.  I think it can be done with a loop
    to test various custom strings, which may begin like ...

	for kwd in "Id" " Id:" "Id $" "Id: $ $" ":" "$ $"
		cat >expanded-keywords.0 <<-EOF &&
		\$$kwd: 0000000000000000000000000000000000000000 \$
		\$$kwd: NoSpaceAtEnd\$
		\$$kwd:NoSpaceAtFront \$
		\$$kwd: NoTerminatingSymbol
		\$$kwd: Foreign Commit With Spaces \$

If we can do without restriction, that would be even better, but I
do not think it is the end of the world if we found some corner case
that a certain customized placeholder can make the syntax ambiguous
and we cannot recognise expanded keywords in the working tree files,
as it is reasonable to limit the letters we allow in placeholders.

One thing we should not do is to tell users that they can use
anything as their customized Id string, and then later find such a
corner case and tell them now we forbid certain letters.  To avoid
such a mistake, it is better to start reasonably tight, allowing
what we know will never cause problems (like alphanumerics) and
forbidding what we know will not be missed in real-world usecases
(like whitespaces and punctuations).  After we discover that certain
things that we initially forbid have real uses in the future, we can
loosen the restriction.  "Start loose and then tighten" will lead to
a regression when viewed from the end-user's workflow.  "Start tight
and then loosen" will not have that problem.

  reply	other threads:[~2021-09-02 19:04 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-23 16:41 [PATCH] Make ident dynamic, not just a hardcoded value of "$Id" Maksym Sobolyev via GitGitGadget
2021-08-23 18:10 ` Junio C Hamano
2021-08-23 18:41 ` Philip Oakley
2021-08-26  4:28 ` [PATCH v2] " Maksym Sobolyev via GitGitGadget
2021-08-26 20:37   ` Matheus Tavares
2021-09-02  0:58     ` Junio C Hamano
2021-09-02 19:04       ` Junio C Hamano [this message]
2021-08-27  2:59   ` Junio C Hamano
     [not found]     ` <>
2021-09-01  5:35       ` Junio C Hamano
2021-09-01  2:13   ` [PATCH v3] " Maksym Sobolyev via GitGitGadget
2021-09-02  3:40     ` Đoàn Trần Công Danh

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:

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

  git send-email \
    --in-reply-to=xmqqmtoukdec.fsf@gitster.g \ \ \ \ \ \ \ \

* 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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.