[v4,1/4] credential-store: document the file format a bit more
diff mbox series

Message ID 20200428105254.28658-1-carenas@gmail.com
State New
Headers show
Series
  • credential-store: prevent fatal errors
Related show

Commit Message

Carlo Marcelo Arenas Belón April 28, 2020, 10:52 a.m. UTC
From: Junio C Hamano <gitster@pobox.com>

Reading a malformed credential URL line and silently ignoring it
does not mean that we promise to torelate and/or keep empty lines
and "# commented" lines forever.

Some people seem to take anything that is not explicitly forbidden
as allowed, but the world does not work that way.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-credential-store.txt | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Eric Sunshine April 28, 2020, 4:06 p.m. UTC | #1
On Tue, Apr 28, 2020 at 6:53 AM Carlo Marcelo Arenas Belón
<carenas@gmail.com> wrote:
> Reading a malformed credential URL line and silently ignoring it
> does not mean that we promise to torelate and/or keep empty lines
> and "# commented" lines forever.
>
> Some people seem to take anything that is not explicitly forbidden
> as allowed, but the world does not work that way.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt
> @@ -94,6 +94,10 @@ stored on its own line as a URL like:
> +No other kinds of lines (e.g. empty lines or comment lines) are
> +allowed in the file, even though some may be silently ignored. Do
> +not view or edit the file with editors.

I suggest dropping the "even though some may be silently ignored" bit
since it's both mysterious (providing no concrete information) and
unnecessarily confusing since it flat out contradicts the earlier part
of the sentence. The fact that the implementation has accidentally
been loose in its parsing doesn't warrant introducing such ambiguity
into the (newly-added) documentation.
Junio C Hamano April 28, 2020, 6:15 p.m. UTC | #2
Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:

> From: Junio C Hamano <gitster@pobox.com>
>
> Reading a malformed credential URL line and silently ignoring it
> does not mean ...

I'd reuse the one I've already queued as 272281ef (credential-store:
document the file format a bit more, 2020-04-27) for this step.

Thanks.

-- >8 --
Subject: [PATCH] credential-store: document the file format a bit more

Reading a malformed credential URL line and silently ignoring it
does not mean that we support empty lines and/or "# commented" lines
forever.  We should document it to avoid confusion.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-credential-store.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt
index 693dd9d9d7..76b0798856 100644
--- a/Documentation/git-credential-store.txt
+++ b/Documentation/git-credential-store.txt
@@ -94,6 +94,10 @@ stored on its own line as a URL like:
 https://user:pass@example.com
 ------------------------------
 
+No other kinds of lines (e.g. empty lines or comment lines) are
+allowed in the file, even though some may be silently ignored. Do
+not view or edit the file with editors.
+
 When Git needs authentication for a particular URL context,
 credential-store will consider that context a pattern to match against
 each entry in the credentials file.  If the protocol, hostname, and
Junio C Hamano April 28, 2020, 6:18 p.m. UTC | #3
Eric Sunshine <sunshine@sunshineco.com> writes:

> On Tue, Apr 28, 2020 at 6:53 AM Carlo Marcelo Arenas Belón
> <carenas@gmail.com> wrote:
>> Reading a malformed credential URL line and silently ignoring it
>> does not mean that we promise to torelate and/or keep empty lines
>> and "# commented" lines forever.
>>
>> Some people seem to take anything that is not explicitly forbidden
>> as allowed, but the world does not work that way.
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt
>> @@ -94,6 +94,10 @@ stored on its own line as a URL like:
>> +No other kinds of lines (e.g. empty lines or comment lines) are
>> +allowed in the file, even though some may be silently ignored. Do
>> +not view or edit the file with editors.
>
> I suggest dropping the "even though some may be silently ignored" bit
> since it's both mysterious (providing no concrete information) and
> unnecessarily confusing since it flat out contradicts the earlier part
> of the sentence. The fact that the implementation has accidentally
> been loose in its parsing doesn't warrant introducing such ambiguity
> into the (newly-added) documentation.

I do not think it is an ambiguity.  The phrase is there just to
remind the readers that they are not allowed to take a loose
implementation in the past as an excuse to throw in crufts and
expect the result to continue working.

Patch
diff mbox series

diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt
index 693dd9d9d7..76b0798856 100644
--- a/Documentation/git-credential-store.txt
+++ b/Documentation/git-credential-store.txt
@@ -94,6 +94,10 @@  stored on its own line as a URL like:
 https://user:pass@example.com
 ------------------------------
 
+No other kinds of lines (e.g. empty lines or comment lines) are
+allowed in the file, even though some may be silently ignored. Do
+not view or edit the file with editors.
+
 When Git needs authentication for a particular URL context,
 credential-store will consider that context a pattern to match against
 each entry in the credentials file.  If the protocol, hostname, and