Hi,
On Sun, 3 Feb 2008, Steffen Prohaska wrote:
> diff --git a/convert.c b/convert.c
> index 80f114b..d580a62 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -95,9 +95,6 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
> return 0;
>
> gather_stats(src, len, &stats);
> - /* No CR? Nothing to convert, regardless. */
> - if (!stats.cr)
> - return 0;
Yes, you add it later, but would this not be "more correct" if you
prepended a "!checksafe &&" and kept the if here? Of course, you'd _also_
need it later, but then _inside_ the "if (checksafe)" clause.
> @@ -115,6 +112,30 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
> return 0;
> }
>
> + if (checksafe) {
> + if (action == CRLF_INPUT || auto_crlf <= 0) {
> + /* CRLFs would not be restored by checkout: check if we'd remove CRLFs */
There are lines in your patch that are substantially longer than 80
characters / line. Please fix.
> diff --git a/environment.c b/environment.c
> index 18a1c4e..e351e99 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -35,6 +35,7 @@ int pager_use_color = 1;
> char *editor_program;
> char *excludes_file;
> int auto_crlf = 0; /* 1: both ways, -1: only when adding git objects */
> +enum safe_crlf safe_crlf = SAFE_CRLF_WARN;
I think this choice needs some defending. At first I thought: "no way
that this will be the default; it affects too many users it should _not_
affect". The thing is, Unix users should not need to suffer, only because
other users are cursed by insane choices of their OS developers.
However, safe_crlf != SAFE_CRLF_FALSE does not affect people who did not
set core.crlf = input or core.crlf = true. And for those who set
core.crlf, the default makes sense, absolutely.
So I think this patch should go in, with shortened lines (and possibly the
changes to the "if (!stats.cr)" if you agree with me).
Ciao,
Dscho
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
