GNU bug report logs - #67118
[PATCH] Change "tabulation" to "tabulation character" in linter & manual

Previous Next

To reply to this bug, email your comments to 67118 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: guix-patches <at> gnu.org
Cc: Ian Eure <ian <at> retrospec.tv>
Subject: [PATCH] Change "tabulation" to "tabulation character" in linter &
 manual
Date: Sat, 11 Nov 2023 15:54:32 -0800

While linting my packages for my first contribution to Guix, I found this
linter warning to be unclear.  The linter looks for ASCII horizontal tab
characters, but the message doesn’t make this clear, as it only says
"tabulation."  Tabulation is the aligning of text into tables, and this may be
accomplished without using ASCII 0x09, for example, with spaces.

This patch clarifies the message to specifically indicate the objectionable
character, updates the test, and changes the language in the manual.

Change-Id: I375d1aa0aec7dfab7e8dfaffb8d4ad0e4f330205
---
 doc/guix.texi  |  3 ++-
 guix/lint.scm  | 19 ++++++++++---------
 tests/lint.scm | 19 ++++++++++---------
 3 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 94903fb5e2..6396f129a1 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -119,6 +119,7 @@
 Copyright @copyright{} 2023 Zheng Junjie@*
 Copyright @copyright{} 2023 Brian Cully@*
 Copyright @copyright{} 2023 Felix Lechner@*
+Copyright @copyright{} 2023 Ian Eure@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -15362,7 +15363,7 @@ Invoking guix lint
 
 @item formatting
 Warn about obvious source code formatting issues: trailing white space,
-use of tabulations, etc.
+use of ASCII tabulation characters, etc.
 
 @item input-labels
 Report old-style input labels that do not match the name of the
diff --git a/guix/lint.scm b/guix/lint.scm
index 861e352b93..f053716abf 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -14,6 +14,7 @@
 ;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
 ;;; Copyright © 2021-2023 Maxime Devos <maximedevos <at> telenet.be>
 ;;; Copyright © 2021 Brice Waegeneire <brice <at> waegenei.re>
+;;; Copyright © 2023 Ian Eure <ian <at> retrospec.tv>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -348,17 +349,17 @@ (define (check-compiler-for-target package)
       (_ '())))
   (parameterize ((%current-target-system "aarch64-linux-gnu"))
     (apply (lambda* (#:key (target 'not-set)
-		     make-flags #:allow-other-keys)
+                     make-flags #:allow-other-keys)
              (define make-flags/sexp
                (if (gexp? make-flags/sexp)
                    (gexp->approximate-sexp make-flags)
                    make-flags))
-	     ;; Some packages like 'tzdata' are never cross-compiled;
-	     ;; the compilers are only used to build tools for
-	     ;; compiling the rest of the package.
-	     (if (eq? target '#false)
-		 '()
-		 (find-incorrect-compilers make-flags/sexp)))
+             ;; Some packages like 'tzdata' are never cross-compiled;
+             ;; the compilers are only used to build tools for
+             ;; compiling the rest of the package.
+             (if (eq? target '#false)
+                 '()
+                 (find-incorrect-compilers make-flags/sexp)))
            (package-arguments package))))
 
 (define (properly-starts-sentence? s)
@@ -1774,12 +1775,12 @@ (define (check-haskell-stackage package)
 ;;;
 
 (define (report-tabulations package line line-number)
-  "Warn about tabulations found in LINE."
+  "Warn about ASCII tabulation characters found in LINE."
   (match (string-index line #\tab)
     (#f #f)
     (index
      (make-warning package
-                   (G_ "tabulation on line ~a, column ~a")
+                   (G_ "tabulation character (0x09) on line ~a, column ~a")
                    (list line-number index)
                    #:location
                    (location (package-file package)
diff --git a/tests/lint.scm b/tests/lint.scm
index a52a82237b..cec9e3ab21 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -11,6 +11,7 @@
 ;;; Copyright © 2020 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
 ;;; Copyright © 2021, 2023 Maxime Devos <maximedevos <at> telenet.be>
+;;; Copyright © 2023 Ian Eure <ian <at> retrospec.tv>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -359,18 +360,18 @@ (define (warning-contains? str warnings)
   '()
   (check-compiler-for-target
    (dummy-package "x"
-		  (arguments
-		   (list #:make-flags
-			 #~(list (string-append "CC=" (cc-for-target))))))))
+                  (arguments
+                   (list #:make-flags
+                         #~(list (string-append "CC=" (cc-for-target))))))))
 
 (test-equal "compiler-for-target: CC=gcc is acceptable when target=#false"
   '()
   (check-compiler-for-target
    ;; This (dummy) package consists purely of architecture-independent data.
    (dummy-package "tzdata"
-		  (arguments
-		   (list #:target #false
-			 #:make-flags #~(list "CC=gcc"))))))
+                  (arguments
+                   (list #:target #false
+                         #:make-flags #~(list "CC=gcc"))))))
 
 ;; The emacs-build-system sets #:tests? #f by default.
 (test-equal "tests-true: #:tests? #t acceptable for emacs packages"
@@ -1307,16 +1308,16 @@ (define (package-with-phase-changes changes)
 
 (test-assert "formatting: tabulation"
   (string-match-or-error
-   "tabulation on line [0-9]+, column [0-9]+"
+   "tabulation character \\(0x09\\) on line [0-9]+, column [0-9]+"
    (single-lint-warning-message
-    (check-formatting (dummy-package "leave the tab here:	")))))
+    (check-formatting (dummy-package "leave the tab here:       ")))))
 
 (test-assert "formatting: trailing white space"
   (string-match-or-error
    "trailing white space .*"
    ;; Leave the trailing white space on the next line!
    (single-lint-warning-message
-    (check-formatting (dummy-package "x")))))            
+    (check-formatting (dummy-package "x")))))
 
 (test-assert "formatting: long line"
   (string-match-or-error

base-commit: af6105afc67a15a491a0a4fd18a28c9f801a0b94
-- 
2.41.0

Message #8 received at 67118 <at> debbugs.gnu.org (full text, mbox):

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Ian Eure <ian <at> retrospec.tv>, 67118 <at> debbugs.gnu.org
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Ian Eure <ian <at> retrospec.tv>,
 Mathieu Othacehe <othacehe <at> gnu.org>,
 Ludovic Courtès <ludo <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#67118] [PATCH] Change "tabulation" to "tabulation
 character" in linter & manual
Date: Fri, 12 Jan 2024 10:03:24 +0100

Hi,

On Sat, 11 Nov 2023 at 15:54, Ian Eure <ian <at> retrospec.tv> wrote:

> diff --git a/doc/guix.texi b/doc/guix.texi
> index 94903fb5e2..6396f129a1 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -119,6 +119,7 @@
>  Copyright @copyright{} 2023 Zheng Junjie@*
>  Copyright @copyright{} 2023 Brian Cully@*
>  Copyright @copyright{} 2023 Felix Lechner@*
> +Copyright @copyright{} 2023 Ian Eure@*

I am not sure that a oneline change implies a Copyright header. :-)

And other change neither, I guess.
  
> -		     make-flags #:allow-other-keys)
> +                     make-flags #:allow-other-keys)
>               (define make-flags/sexp
>                 (if (gexp? make-flags/sexp)
>                     (gexp->approximate-sexp make-flags)
>                     make-flags))
> -	     ;; Some packages like 'tzdata' are never cross-compiled;
> -	     ;; the compilers are only used to build tools for
> -	     ;; compiling the rest of the package.
> -	     (if (eq? target '#false)
> -		 '()
> -		 (find-incorrect-compilers make-flags/sexp)))
> +             ;; Some packages like 'tzdata' are never cross-compiled;
> +             ;; the compilers are only used to build tools for
> +             ;; compiling the rest of the package.
> +             (if (eq? target '#false)
> +                 '()
> +                 (find-incorrect-compilers make-flags/sexp)))
>             (package-arguments package))))

There is no change here, right?

> diff --git a/tests/lint.scm b/tests/lint.scm
> index a52a82237b..cec9e3ab21 100644
> --- a/tests/lint.scm
> +++ b/tests/lint.scm
> @@ -11,6 +11,7 @@
>  ;;; Copyright © 2020 Tobias Geerinckx-Rice <me <at> tobias.gr>
>  ;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
>  ;;; Copyright © 2021, 2023 Maxime Devos <maximedevos <at> telenet.be>
> +;;; Copyright © 2023 Ian Eure <ian <at> retrospec.tv>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -359,18 +360,18 @@ (define (warning-contains? str warnings)
>    '()
>    (check-compiler-for-target
>     (dummy-package "x"
> -		  (arguments
> -		   (list #:make-flags
> -			 #~(list (string-append "CC=" (cc-for-target))))))))
> +                  (arguments
> +                   (list #:make-flags
> +                         #~(list (string-append "CC=" (cc-for-target))))))))

No change here, right?

>  (test-equal "compiler-for-target: CC=gcc is acceptable when target=#false"
>    '()
>    (check-compiler-for-target
>     ;; This (dummy) package consists purely of architecture-independent data.
>     (dummy-package "tzdata"
> -		  (arguments
> -		   (list #:target #false
> -			 #:make-flags #~(list "CC=gcc"))))))
> +                  (arguments
> +                   (list #:target #false
> +                         #:make-flags #~(list "CC=gcc"))))))

No change?

>  ;; The emacs-build-system sets #:tests? #f by default.
>  (test-equal "tests-true: #:tests? #t acceptable for emacs packages"
> @@ -1307,16 +1308,16 @@ (define (package-with-phase-changes changes)
>  
>  (test-assert "formatting: tabulation"
>    (string-match-or-error
> -   "tabulation on line [0-9]+, column [0-9]+"
> +   "tabulation character \\(0x09\\) on line [0-9]+, column [0-9]+"
>     (single-lint-warning-message
> -    (check-formatting (dummy-package "leave the tab here:	")))))
> +    (check-formatting (dummy-package "leave the tab here:       ")))))
>  
>  (test-assert "formatting: trailing white space"
>    (string-match-or-error
>     "trailing white space .*"
>     ;; Leave the trailing white space on the next line!
>     (single-lint-warning-message
> -    (check-formatting (dummy-package "x")))))            
> +    (check-formatting (dummy-package "x")))))

What is the change here?


Cheers,
simon

Message #11 received at 67118 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: Simon Tournier <zimon.toutoune <at> gmail.com>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Ludovic Courtès <ludo <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 67118 <at> debbugs.gnu.org, Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#67118] [PATCH] Change "tabulation" to "tabulation
 character" in linter & manual
Date: Sat, 13 Jan 2024 13:25:46 -0800

Simon Tournier <zimon.toutoune <at> gmail.com> writes:

> Hi,
>
> On Sat, 11 Nov 2023 at 15:54, Ian Eure <ian <at> retrospec.tv> wrote:
>
>> diff --git a/doc/guix.texi b/doc/guix.texi
>> index 94903fb5e2..6396f129a1 100644
>> --- a/doc/guix.texi
>> +++ b/doc/guix.texi
>> @@ -119,6 +119,7 @@
>>  Copyright @copyright{} 2023 Zheng Junjie@*
>>  Copyright @copyright{} 2023 Brian Cully@*
>>  Copyright @copyright{} 2023 Felix Lechner@*
>> +Copyright @copyright{} 2023 Ian Eure@*
>
> I am not sure that a oneline change implies a Copyright 
> header. :-)
>

Sure, I can remove that.


> And other change neither, I guess.
>   
>> -		     make-flags #:allow-other-keys)
>> +                     make-flags #:allow-other-keys)
>>               (define make-flags/sexp
>>                 (if (gexp? make-flags/sexp)
>>                     (gexp->approximate-sexp make-flags)
>>                     make-flags))
>> -	     ;; Some packages like 'tzdata' are never 
>> cross-compiled;
>> -	     ;; the compilers are only used to build tools for
>> -	     ;; compiling the rest of the package.
>> -	     (if (eq? target '#false)
>> -		 '()
>> -		 (find-incorrect-compilers make-flags/sexp)))
>> +             ;; Some packages like 'tzdata' are never 
>> cross-compiled;
>> +             ;; the compilers are only used to build tools for
>> +             ;; compiling the rest of the package.
>> +             (if (eq? target '#false)
>> +                 '()
>> +                 (find-incorrect-compilers make-flags/sexp)))
>>             (package-arguments package))))
>
> There is no change here, right?
>

I believe the whitespace changes were a result of running `guix 
style' or similar, I definitely didn’t intend for the patch to 
contain all that.  I’ll back those out and send a new one.

Message #14 received at 67118 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: 67118 <at> debbugs.gnu.org
Cc: Ian Eure <ian <at> retrospec.tv>
Subject: [PATCH 1/1] Update text to refer to "tabulation character."
Date: Sat, 13 Jan 2024 16:48:33 -0800

Change-Id: I8de95149502e6cb1458e9f4102895395865d7000
---
 doc/guix.texi  | 2 +-
 guix/lint.scm  | 4 ++--
 tests/lint.scm | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index a648a106b3..bf9b1fd5aa 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -15513,7 +15513,7 @@ declare them as in this example:
 
 @item formatting
 Warn about obvious source code formatting issues: trailing white space,
-use of tabulations, etc.
+use of ASCII tabulation characters, etc.
 
 @item input-labels
 Report old-style input labels that do not match the name of the
diff --git a/guix/lint.scm b/guix/lint.scm
index 861e352b93..fbdf1e5b9f 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -1774,12 +1774,12 @@ (define (check-haskell-stackage package)
 ;;;
 
 (define (report-tabulations package line line-number)
-  "Warn about tabulations found in LINE."
+  "Warn about ASCII tabulation characters found in LINE."
   (match (string-index line #\tab)
     (#f #f)
     (index
      (make-warning package
-                   (G_ "tabulation on line ~a, column ~a")
+                   (G_ "tabulation character (0x09) on line ~a, column ~a")
                    (list line-number index)
                    #:location
                    (location (package-file package)
diff --git a/tests/lint.scm b/tests/lint.scm
index a52a82237b..75afd3ecdb 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -1307,7 +1307,7 @@ (define (package-with-phase-changes changes)
 
 (test-assert "formatting: tabulation"
   (string-match-or-error
-   "tabulation on line [0-9]+, column [0-9]+"
+   "tabulation character \\(0x09\\) on line [0-9]+, column [0-9]+"
    (single-lint-warning-message
     (check-formatting (dummy-package "leave the tab here:	")))))
 
-- 
2.41.0

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.