On Mon, Jan 6, 2025 at 9:52 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
Hi Junio,
> Usman Akinyemi <usmanakinyemi202@xxxxxxxxx> writes:
>
> > +
> > +transfer.advertiseOSVersion::
> > + When `true`, the `os-version` capability is advertised by clients and
> > + servers. It makes clients and servers send to each other a string
> > + representing the operating system name, like "Linux" or "Windows".
> > + This string is retrieved from the 'sysname' field of the struct returned
> > + by the uname(2) system call. Defaults to true.
>
> Shouldn't `sysname` be typeset as a literal, just like `true` and
> `os-version`?
I will do that in the next iteration. Thank you.
>
> > +os-version
> > +~~~~~~~~~~
> > +
> > +In the same way as the `agent` capability above, the server can
> > +advertise the `os-version` capability with a value `X` (in the form
> > +`os-version=X`) to notify the client that the server is running an
> > +operating system that can be identified by `X`. The client may
>
> Hmph. I am not sure what's the value of mentioning 'X' here. To me
>
> ... can advertise the `os-version` capability to notify the kind
> of operating system it is running on.
>
> conveys the same thing with much fewer bytes.
Yeah, it is better, I will use it in the next iteration.
>
> > +optionally send its own `os-version` string by including the
> > +`os-version` capability with a value `Y` (in the form `os-version=Y`)
> > +in its request to the server (but it MUST NOT do so if the server did
> > +not advertise the os-version capability). The `X` and `Y` strings may
> > +contain any printable ASCII characters except space (i.e., the byte
>
> This is misleading. ASCII printable characters range from 33 to 126
> (inclusive), but by saying "except space", the readers are led to
> believe that the author of this documentation thinks ASCII 32 is
> printable, too.
Thanks for this, I will make changes in the next iteration.
>
> About 'X' and 'Y', we can just say "the value of this capability may
> consist of ASCII printable characters (from 33 to 126 inclusive)" or
> something.
>
Noted. Thank you.
> Is there a need for a registry of canonical os-version strings? One
> reason why you would want this user-settable (as opposed to being
> derived from "uname -s") is that a system that is presumably the
> same in end-user perception can call itself in different names (your
> Windows/MINGW64 example) and having the users set it to a string
> chosen from a small repertoire, the other end would be able to
> identify them more easily. I do not think it is a necessarily a
> good idea to limit what value the users can set to this
> configuration variable, but at least with a published guideline on
> calling various types of systems (and an explanation on the reason
> why we publish such a guideline), users would make an informed
> decision when picking what string to send.
We plan to implement another config option `osVersion.format`, which
allow users to fully customize the string sent to the other side using
placeholders,
similar to how git for-each-ref uses %() syntax. The user would be
able to set it to
the string they want i.e "Linux" or "Windows" (without any
placeholder) and would be
sent as-is. So, the `osVersion.format` should satisfy this need. I
will ensure to document
this option to tell that it can be used like this and will give a
small list of `os-version` strings
that can be used in this way.
>
> > +# Trim and replace each character with ascii code below 32 or above
> > +# 127 (included) using a dot '.' character.
> > +# Octal intervals \001-\040 and \177-\377
> > +# corresponds to decimal intervals 1-32 and 127-255
> > +test_redact_non_printables () {
> > + tr -d "\n" | tr "[\001-\040][\177-\377]" "."
> > +}
>
> Just being curious. Do we need to worry about carriage-returns not
> just line-feeds, and if not why?
The function `tr "[\001-\040][\177-\377]" "."` already replace the
carriage-returns with "."
the redact_non_printables() will also replace it with ".".
Carriage-returns octal code is 015 and
decimal code of 13. So, we do not need to worry about it.
>
> Thanks.
Thank you.
Usman.
