unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#67118] [PATCH] Change "tabulation" to "tabulation character" in linter & manual
@ 2023-11-11 23:54 Ian Eure
  2024-01-12  9:03 ` Simon Tournier
  2024-01-14  0:48 ` [bug#67118] [PATCH 1/1] Update text to refer to "tabulation character." Ian Eure
  0 siblings, 2 replies; 4+ messages in thread
From: Ian Eure @ 2023-11-11 23:54 UTC (permalink / raw)
  To: 67118
  Cc: Ian Eure, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice

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@yoctocell.xyz>
 ;;; Copyright © 2021-2023 Maxime Devos <maximedevos@telenet.be>
 ;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
+;;; Copyright © 2023 Ian Eure <ian@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@tobias.gr>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
 ;;; Copyright © 2021, 2023 Maxime Devos <maximedevos@telenet.be>
+;;; Copyright © 2023 Ian Eure <ian@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





^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [bug#67118] [PATCH] Change "tabulation" to "tabulation character" in linter & manual
  2023-11-11 23:54 [bug#67118] [PATCH] Change "tabulation" to "tabulation character" in linter & manual Ian Eure
@ 2024-01-12  9:03 ` Simon Tournier
  2024-01-13 21:25   ` Ian Eure
  2024-01-14  0:48 ` [bug#67118] [PATCH 1/1] Update text to refer to "tabulation character." Ian Eure
  1 sibling, 1 reply; 4+ messages in thread
From: Simon Tournier @ 2024-01-12  9:03 UTC (permalink / raw)
  To: Ian Eure, 67118
  Cc: Josselin Poiret, Ian Eure, Mathieu Othacehe, Ludovic Courtès,
	Tobias Geerinckx-Rice, Ricardo Wurmus, Christopher Baines

Hi,

On Sat, 11 Nov 2023 at 15:54, Ian Eure <ian@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@tobias.gr>
>  ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
>  ;;; Copyright © 2021, 2023 Maxime Devos <maximedevos@telenet.be>
> +;;; Copyright © 2023 Ian Eure <ian@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




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [bug#67118] [PATCH] Change "tabulation" to "tabulation character" in linter & manual
  2024-01-12  9:03 ` Simon Tournier
@ 2024-01-13 21:25   ` Ian Eure
  0 siblings, 0 replies; 4+ messages in thread
From: Ian Eure @ 2024-01-13 21:25 UTC (permalink / raw)
  To: Simon Tournier
  Cc: Josselin Poiret, Mathieu Othacehe, Ludovic Courtès,
	Tobias Geerinckx-Rice, Ricardo Wurmus, 67118, Christopher Baines


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

> Hi,
>
> On Sat, 11 Nov 2023 at 15:54, Ian Eure <ian@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.




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [bug#67118] [PATCH 1/1] Update text to refer to "tabulation character."
  2023-11-11 23:54 [bug#67118] [PATCH] Change "tabulation" to "tabulation character" in linter & manual Ian Eure
  2024-01-12  9:03 ` Simon Tournier
@ 2024-01-14  0:48 ` Ian Eure
  1 sibling, 0 replies; 4+ messages in thread
From: Ian Eure @ 2024-01-14  0:48 UTC (permalink / raw)
  To: 67118; +Cc: Ian Eure

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





^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-01-14  0:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-11 23:54 [bug#67118] [PATCH] Change "tabulation" to "tabulation character" in linter & manual Ian Eure
2024-01-12  9:03 ` Simon Tournier
2024-01-13 21:25   ` Ian Eure
2024-01-14  0:48 ` [bug#67118] [PATCH 1/1] Update text to refer to "tabulation character." Ian Eure

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.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).