git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 0/5] Abide by our own rules regarding line endings
Date: Thu, 04 May 2017 08:04:26 -0700	[thread overview]
Message-ID: <xmqq60hgacjp.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <alpine.DEB.2.21.1.1705041129170.4905@virtualbox> (Johannes Schindelin's message of "Thu, 4 May 2017 11:47:03 +0200 (CEST)")

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Well, a couple of comments about your comment:
>
> - we say "shell scripts", but we're sloppy there: they are "Unix shell
>   scripts", as they are executed by Unix shells. As such, it is pretty
>   obvious that they favor Unix line endings, right? And that they do not
>   really handle anything else well, right?

Not really.  I would expect that

	cat A B C

with CRLF line endings (i.e. "cat A B C<CR><LF>") on platforms whose
eol convention is LF to barf due to the missing file whose name is
"C<CR>", while on platforms whose eol convention is CRLF, I do
expect the contents of file C comes at the end of the output.

> - You try to say that it is not okay for shell scripts to be checked
>   out as LF-only when the platform convention for *text* files is CR/LF,
>   right? 

I didn't try to and I didn't say that, I hope.  I think it is not OK
for shell scripts to be checked out with <CR><LF> when the platform
convention for text is <LF>, though.  It may be possible for an
implementation on CRLF platform to tolerate missing <CR> (i.e. a
file in LF convention--instead of treating a lone <LF> as a non-eol
but just a regular "funny" byte, treat it as a normal eol), but that
would be a quality-of-implementation thing.  So when the platform
convention for text files is <CR><LF>, I would have thought that it
was more natural to check out shell scripts as such.

However, I did not think things through when I said "I am not sure
if it is OK for shell scripts not to honor the platform convention,
though."  In a sense, the "cat A B C" example does not have to worry
about the platform convention issue very much.  In real scripts, we
have things like a string literal that spans lines (i.e. do we have
a LF, or a CRLF pair, in between these two lines?) and here
documents (ditto).  To handle these "correctly" while treating shell
scripts mere "text" files, a shell implementation on CRLF platform
has to accept CRLF, pretend as if it saw only LF, do all the
processing as if the eol convention were LF, and convert LF in the
output from the script to CRLF.  I think that is _too_ much to ask
for an implementation, and such an attempt would get corner cases
wrong.  IOW, I was not sure if it is OK for scripts not to honor the
platorm convention when I wrote the message you are responding to,
but I think it probably is not just OK but is the simplest/cleanest
for shell implementations not to honor the platform convention and
instead pretend that they always live in LF-only world.

And all of the above ultimately does not matter.  If the shell you
have on Windows cannot take CRLF files, then our shell script must
be checked out as LF files even on Windows.  My "I am not sure" was
mostly from not knowing how ingrained that "cannot take CRLF files"
was (i.e. I didn't know if there may be some knobs similar to
core.crlf we have that you can toggle for your shell to honor the
platform convention).



  reply	other threads:[~2017-05-04 15:05 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-02 12:29 [PATCH 0/5] Abide by our own rules regarding line endings Johannes Schindelin
2017-05-02 12:30 ` [PATCH 1/5] Fix build with core.autocrlf=true Johannes Schindelin
2017-05-02 12:30 ` [PATCH 2/5] git-new-workdir: mark script as LF-only Johannes Schindelin
2017-05-02 12:30 ` [PATCH 3/5] completion: mark bash " Johannes Schindelin
2017-05-02 12:30 ` [PATCH 4/5] Fix the remaining tests that failed with core.autocrlf=true Johannes Schindelin
2017-05-02 12:30 ` [PATCH 5/5] t4051: mark supporting files as requiring LF-only line endings Johannes Schindelin
2017-05-02 21:56 ` [PATCH 0/5] Abide by our own rules regarding " Jonathan Nieder
2017-05-03 14:23   ` Johannes Schindelin
2017-05-04  5:19 ` Junio C Hamano
2017-05-04  9:47   ` Johannes Schindelin
2017-05-04 15:04     ` Junio C Hamano [this message]
2017-05-04 18:48       ` Johannes Schindelin
2017-05-07  5:29         ` Junio C Hamano
2017-05-08 10:49           ` Johannes Schindelin
2017-05-04  9:49 ` [PATCH v2 0/7] " Johannes Schindelin
2017-05-04  9:49   ` [PATCH v2 1/7] Fix build with core.autocrlf=true Johannes Schindelin
2017-05-08  5:08     ` Junio C Hamano
2017-05-04  9:49   ` [PATCH v2 2/7] git-new-workdir: mark script as LF-only Johannes Schindelin
2017-05-08  5:11     ` Junio C Hamano
2017-05-04  9:49   ` [PATCH v2 3/7] completion: mark bash " Johannes Schindelin
2017-05-04  9:49   ` [PATCH v2 4/7] t3901: move supporting files into t/t3901/ Johannes Schindelin
2017-05-08  5:12     ` Junio C Hamano
2017-05-04  9:50   ` [PATCH v2 5/7] t4003, t4005, t4007 & t4008: handle CR/LF in t/README & t/diff-lib/README Johannes Schindelin
2017-05-08  5:14     ` Junio C Hamano
2017-05-04  9:50   ` [PATCH v2 6/7] Fix the remaining tests that failed with core.autocrlf=true Johannes Schindelin
2017-05-08  5:22     ` Junio C Hamano
2017-05-04  9:50   ` [PATCH v2 7/7] t4051: mark supporting files as requiring LF-only line endings Johannes Schindelin
2017-05-08  5:29     ` Junio C Hamano
2017-05-09 11:20       ` Johannes Schindelin
2017-05-09 12:53   ` [PATCH v3 0/6] Abide by our own rules regarding " Johannes Schindelin
2017-05-09 12:53     ` [PATCH v3 1/6] Fix build with core.autocrlf=true Johannes Schindelin
2017-05-22 17:57       ` Jonathan Nieder
2017-05-23  3:35         ` Junio C Hamano
2017-05-23  9:01           ` Johannes Schindelin
2017-05-09 12:53     ` [PATCH v3 2/6] git-new-workdir: mark script as LF-only Johannes Schindelin
2017-05-09 12:54     ` [PATCH v3 3/6] completion: mark bash " Johannes Schindelin
2017-05-09 12:54     ` [PATCH v3 4/6] t3901: move supporting files into t/t3901/ Johannes Schindelin
2017-05-09 12:54     ` [PATCH v3 5/6] Fix the remaining tests that failed with core.autocrlf=true Johannes Schindelin
2017-05-09 12:54     ` [PATCH v3 6/6] t4051: mark supporting files as requiring LF-only line endings Johannes Schindelin

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqq60hgacjp.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).