Re: [PATCH v3] ls-files: Add eol diagnostics
On 23/11/15 17:05, Torsten Bögershausen wrote: > On 22.11.15 09:20, Sebastian Schuberth wrote: >> On 21.11.2015 08:36, Torsten Bögershausen wrote: >> >>> git ls-files --eol gives an output like this: >>> >>> i/text-no-eol w/text-no-eol attr/text=auto t/t5100/empty >> >> I'm sorry if this has been discussed before, but hav you considered to use a >> header line and omit the prefixed from the columns instead? Like >> >> index working tree attributesfile >> >> binarybinary -text t/test-binary-2.png >> text-lf text-lf eol=lft/t5100/rfc2047-info-0007 >> text-lf text-crlfeol=crlf doit.bat >> text-crlf-lf text-crlf-lf locale/XX.po >> >> I believe this would be both easier to read for humans, and easier to parse >> for scripts that e.g. want to compare line endings in the index and working >> tree. >> > The problem I see is to make sure that there is always a separator, even when > a field empty: > > rm zlib.c; git ls-file --eol #will include a line like this: > i/text-lf w/ attr/ zlib.c > > or, as another example: > git ls-files -o --eol > i/ w/binaryattr/ zlib.o > [I have been trying *not* to respond to this thread, since the following comment is classic bikeshedding on the naming of things and I am, in general, hopeless at naming things ... :-D ] I don't think you need to include 'text' in the name of the line ending descriptors - if its not binary then its some form of text. So, maybe something like: binary none lf crlf mixed HTH ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] ls-files: Add eol diagnostics
On Mon, Nov 23, 2015 at 2:45 PM, Sebastian Schuberth wrote: > On Mon, Nov 23, 2015 at 6:05 PM, Torsten Bögershausen wrote: >> git ls-files -o --eol >> i/ w/binaryattr/ zlib.o > > I see, somewhat convincing I have to agree. > > On another note, how about making the prefix either all just one > letter (i.e. "attr/" becomes "a/"), or all multi-letter abbreviations > (i.e. "i/" becomes "idx/" and "w/" becomes "wt/" or "wtree/" or > "tree/")? The latter is answered by [1]. [1]: http://article.gmane.org/gmane.comp.version-control.git/280647 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] ls-files: Add eol diagnostics
On Mon, Nov 23, 2015 at 6:05 PM, Torsten Bögershausen wrote: > or, as another example: > git ls-files -o --eol > i/ w/binaryattr/ zlib.o I see, somewhat convincing I have to agree. On another note, how about making the prefix either all just one letter (i.e. "attr/" becomes "a/"), or all multi-letter abbreviations (i.e. "i/" becomes "idx/" and "w/" becomes "wt/" or "wtree/" or "tree/")? -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] ls-files: Add eol diagnostics
On Sat, Nov 21, 2015 at 2:36 AM, Torsten Bögershausen wrote:
> When working in a cross-platform environment, a user wants to
> check if text files are stored normalized in the repository and if
> .gitattributes are set appropriately.
>
> Make it possible to let Git show the line endings in the index and
> in the working tree and the effective text/eol attributes
s/$/./
> [...]
> Signed-off-by: Torsten Bögershausen
> ---
> diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> @@ -147,6 +148,18 @@ a space) at the start of each line:
> possible for manual inspection; the exact format may change at
> any time.
>
> +--eol::
> + Show line endings ("eolinfo") and the text/eol attributes
> ("texteolattr") of files.
> + "eolinfo" is the file content identification used by Git when
> + the "text" attribute is "auto", or core.autocrlf != false.
> +
> + "eolinfo" is either "" (when the the info is not avaiable"), or one
> of "binary",
s/avaiable/available/
> + "text-no-eol", "text-lf", "text-crlf" or "text-crlf-lf".
> + The "texteolattr" can be "", "-text", "text", "text=auto", "eol=lf",
> "eol=crlf".
> +
> + Both the content in the index ("i/") and the content in the working
> tree ("w/")
> + are shown for regular files, followed by the "exteolattr ("attr/").
s/exteolattr/texteolattr/
Should the documentation mention that with --eol ls-files becomes a
much more expensive operation since it now has to read all blobs
referenced by trees? (genuine question)
> diff --git a/builtin/check-attr.c b/builtin/check-attr.c
> @@ -51,6 +53,23 @@ static void output_attr(int cnt, struct git_attr_check
> *check,
> +static void output_eol_attr(const char *file)
> +{
> + struct stat st;
> + if (lstat(file, &st) || (!S_ISREG(st.st_mode)))
> + return;
> + if (nul_term_line) {
> + printf("%s:", file);
> + if (!lstat(file, &st) && (S_ISREG(st.st_mode)))
> + printf(": %-13s ", get_convert_attr_ascii(file));
Why the trailing space in the format string?
> + printf("%c", 0);
putc('\0') perhaps?
> + } else {
> + quote_c_style(file, NULL, stdout, 0);
> + printf(": %-13s ", get_convert_attr_ascii(file));
Ditto regarding trailing space.
> + printf("\n");
Is there a reason the "\n" isn't incorporated into the preceding
printf's format string?
> + }
> +}
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> @@ -213,7 +238,18 @@ checkout_files () {
> git -c core.eol=$eol checkout $src$f.txt
> fi
> done
> -
> + test_expect_success "ls-files --eol $lfname ${pfx}LF.txt" "
> + echo i/text-lf w/$(stats_ascii $lfname) ${src}LF.txt >e &&
> + echo i/text-crlf w/$(stats_ascii $crlfname) ${src}CRLF.txt
> >>e &&
> + echo i/text-crlf-lf w/$(stats_ascii $lfmixcrlf)
> ${src}CRLF_mix_LF.txt >>e &&
> + echo i/binary w/$(stats_ascii $lfmixcr) ${src}LF_mix_CR.txt
> >>e &&
> + echo i/binary w/$(stats_ascii $crlfnul) ${src}CRLF_nul.txt
> >>e &&
> + echo i/binary w/$(stats_ascii $crlfnul) ${src}LF_nul.txt >>e
> &&
I wonder if a here-doc would make this a bit more readable and maintainable...
echo >e <<-EOF &&
i/text-lf w/$(stats_ascii $lfname) ${src}LF.txt
i/text-crlf w/$(stats_ascii $crlfname) ${src}CRLF.txt
i/text-crlf-lf w/$(stats_ascii $lfmixcrlf) ${src}CRLF_mix_LF.txt
i/binary w/$(stats_ascii $lfmixcr) ${src}LF_mix_CR.txt
i/binary w/$(stats_ascii $crlfnul) ${src}CRLF_nul.txt
i/binary w/$(stats_ascii $crlfnul) ${src}LF_nul.txt
EOF
> + sort exp &&
> + git ls-files --eol $src* | sed -e 's!attr/[=a-z-]*!!g' -e 's/
> */ /g' | sort >act &&
> + test_cmp exp act &&
> + rm e exp act
> + "
> @@ -231,6 +267,35 @@ checkout_files () {
> "
> }
>
> +# Test control characters
> +# NUL SOH CR EOF==^Z
> +test_expect_success 'ls-files --eol -o Text/Binary' '
> + test_when_finished "rm e exp act TeBi_*" &&
> + STRT=AAA &&
> + STR=$STRT$STRT$STRT$STRT &&
> + printf "${STR}BBB\001" >TeBi_127_S &&
> + printf "${STR}\001">TeBi_128_S &&
> + printf "${STR}BBB\032" >TeBi_127_E &&
> + printf "\032${STR}BBB" >TeBi_E_127 &&
> + printf "${STR}\000">TeBi_128_N &&
> + printf "${STR}BBB\012">TeBi_128_L &&
> + printf "${STR}BBB\015">TeBi_127_C &&
> + printf "${STR}BB\015\012" >TeBi_126_CL &&
> + printf "${STR}BB\015\012\015" >TeBi_126_CLC &&
> + echo i/ w/binary TeBi_127_S >e &&
> + echo i/ w/text-no-eol TeBi_128_S >>e &&
> + echo i/ w/text-no-eol TeBi_127_E >>e &&
> + echo i/ w/binary TeBi_E_127 >>e &&
> + echo i/ w/binary TeBi_128_N >>e &&
> + ech
Re: [PATCH v3] ls-files: Add eol diagnostics
On 22.11.15 09:20, Sebastian Schuberth wrote: > On 21.11.2015 08:36, Torsten Bögershausen wrote: > >> git ls-files --eol gives an output like this: >> >> i/text-no-eol w/text-no-eol attr/text=auto t/t5100/empty > > I'm sorry if this has been discussed before, but hav you considered to use a > header line and omit the prefixed from the columns instead? Like > > index working tree attributesfile > > binarybinary -text t/test-binary-2.png > text-lf text-lf eol=lft/t5100/rfc2047-info-0007 > text-lf text-crlfeol=crlf doit.bat > text-crlf-lf text-crlf-lf locale/XX.po > > I believe this would be both easier to read for humans, and easier to parse > for scripts that e.g. want to compare line endings in the index and working > tree. > The problem I see is to make sure that there is always a separator, even when a field empty: rm zlib.c; git ls-file --eol #will include a line like this: i/text-lf w/ attr/ zlib.c or, as another example: git ls-files -o --eol i/ w/binaryattr/ zlib.o And if there is no separator, it is harder to make it machine-parsable, if we e.g. extend the attributes to support "*text=autocrlf", or "*.text=autoinput" (But that is another story) If we replace "/[-a-z]" with "\t", the line has always a separator, but needs a somewhat wider screen: text-lf text-lf zlib.c >> +echo huh $1 [] good catch, thanks -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] ls-files: Add eol diagnostics
On 21.11.2015 08:36, Torsten Bögershausen wrote:
> git ls-files --eol gives an output like this:
>
> i/text-no-eol w/text-no-eol attr/text=auto t/t5100/empty
I'm sorry if this has been discussed before, but hav you considered to use a
header line and omit the prefixed from the columns instead? Like
index working tree attributesfile
binarybinary -text t/test-binary-2.png
text-lf text-lf eol=lft/t5100/rfc2047-info-0007
text-lf text-crlfeol=crlf doit.bat
text-crlf-lf text-crlf-lf locale/XX.po
I believe this would be both easier to read for humans, and easier to parse for
scripts that e.g. want to compare line endings in the index and working tree.
> +stats_ascii () {
> + case "$1" in
[...]
> + *)
> + echo huh $1
> + ;;
Personally, I'm not a big fan of supposedly funny output like this. How about
printing a proper message rather than "huh", even for cases that should not
happen?
--
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] ls-files: Add eol diagnostics
When working in a cross-platform environment, a user wants to
check if text files are stored normalized in the repository and if
.gitattributes are set appropriately.
Make it possible to let Git show the line endings in the index and
in the working tree and the effective text/eol attributes
The end of line ("eolinfo") are shown like this:
"binary" binary file
"text-no-eol" text file without any EOL
"text-lf" text file with LF
"text-crlf"text file with CRLF
"text-crlf-lf" text file with mixed line endings.
The effective text/eol attribute is one of these:
"", "-text", "text", "text=auto", "eol=lf", "eol=crlf"
git ls-files --eol gives an output like this:
i/text-no-eol w/text-no-eol attr/text=auto t/t5100/empty
i/binaryw/binaryattr/-text t/test-binary-2.png
i/text-lf w/text-lf attr/eol=lft/t5100/rfc2047-info-0007
i/text-lf w/text-crlf attr/eol=crlf doit.bat
i/text-crlf-lf w/text-crlf-lf attr/ locale/XX.po
Note that the output is meant to be human-readable and may change.
Signed-off-by: Torsten Bögershausen
---
Changes since v2:
- Major rework
- New prefix, more common to what we use in Git (i/, w/ attr/)
- Only one option: git ls-files --eol (may be combined with -s or -o or -d
- Simplify the diagnostics: either the file is binary or text
As empty files are not binary, they are text-no-eol
- Test cases in t0027
- The documentation is improved, but may need more improving
Documentation/git-ls-files.txt | 22 +
builtin/check-attr.c | 34 --
builtin/ls-files.c | 19
convert.c | 86 ++
convert.h | 3 ++
t/t0027-auto-crlf.sh | 104 -
6 files changed, 252 insertions(+), 16 deletions(-)
diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index e26f01f..7c87a8d 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -12,6 +12,7 @@ SYNOPSIS
'git ls-files' [-z] [-t] [-v]
(--[cached|deleted|others|ignored|stage|unmerged|killed|modified])*
(-[c|d|o|i|s|u|k|m])*
+ [--eol]
[-x |--exclude=]
[-X |--exclude-from=]
[--exclude-per-directory=]
@@ -147,6 +148,18 @@ a space) at the start of each line:
possible for manual inspection; the exact format may change at
any time.
+--eol::
+ Show line endings ("eolinfo") and the text/eol attributes
("texteolattr") of files.
+ "eolinfo" is the file content identification used by Git when
+ the "text" attribute is "auto", or core.autocrlf != false.
+
+ "eolinfo" is either "" (when the the info is not avaiable"), or one of
"binary",
+ "text-no-eol", "text-lf", "text-crlf" or "text-crlf-lf".
+ The "texteolattr" can be "", "-text", "text", "text=auto", "eol=lf",
"eol=crlf".
+
+ Both the content in the index ("i/") and the content in the working
tree ("w/")
+ are shown for regular files, followed by the "exteolattr ("attr/").
+
\--::
Do not interpret any more arguments as options.
@@ -161,6 +174,15 @@ which case it outputs:
[ ]
+'git ls-files --eol' will show
+ i/ w/ attr/
+
+'git ls-files --eol -o' will show
+ i/ w/ attr/
+
+'git ls-files --eol -s' will show
+[ ] i/ w/ attr/
+
'git ls-files --unmerged' and 'git ls-files --stage' can be used to examine
detailed information on unmerged paths.
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 265c9ba..7734715 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -5,6 +5,7 @@
#include "parse-options.h"
static int all_attrs;
+static int eol_attrs;
static int cached_attrs;
static int stdin_paths;
static const char * const check_attr_usage[] = {
@@ -17,6 +18,7 @@ static int nul_term_line;
static const struct option check_attr_options[] = {
OPT_BOOL('a', "all", &all_attrs, N_("report all attributes set on
file")),
+ OPT_BOOL('e', "eol", &eol_attrs, N_("report effective eol attributes
set on file")),
OPT_BOOL(0, "cached", &cached_attrs, N_("use .gitattributes only from
the index")),
OPT_BOOL(0 , "stdin", &stdin_paths, N_("read file names from stdin")),
OPT_BOOL('z', NULL, &nul_term_line,
@@ -51,6 +53,23 @@ static void output_attr(int cnt, struct git_attr_check
*check,
}
}
+static void output_eol_attr(const char *file)
+{
+ struct stat st;
+ if (lstat(file, &st) || (!S_ISREG(st.st_mode)))
+ return;
+ if (nul_term_line) {
+ printf("%s:", file);
+ if (!lstat(file, &st) && (S_ISREG(st.st_mode)))
+ printf(": %-13s ", get_convert_attr_ascii(file));
+ printf("%c", 0);
+ } else {
+ quote_c_style(file, NULL, stdout, 0);
+
