[PATCH] Change "tabulation" to "tabulation character" in linter & manual

  • Open
  • quality assurance status badge
Details
2 participants
  • Ian Eure
  • Simon Tournier
Owner
unassigned
Submitted by
Ian Eure
Severity
normal
I
I
Ian Eure wrote on 12 Nov 2023 00:54
(address . guix-patches@gnu.org)(name . Ian Eure)(address . ian@retrospec.tv)
d025ed90d2d10e3504cca292f55e40ef36c6d4a0.1699746871.git.ian@retrospec.tv
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(-)

Toggle diff (134 lines)
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
S
S
Simon Tournier wrote on 12 Jan 10:03 +0100
875xzyzzxf.fsf@gmail.com
Hi,

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

Toggle quote (10 lines)
> 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.
Toggle quote (20 lines)
> - 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?

Toggle quote (23 lines)
> 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?

Toggle quote (12 lines)
> (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?

Toggle quote (20 lines)
> ;; 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
I
I
Ian Eure wrote on 13 Jan 22:25 +0100
(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
87ttngq5xw.fsf@retrospec.tv
Simon Tournier <zimon.toutoune@gmail.com> writes:

Toggle quote (18 lines)
> 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.


Toggle quote (27 lines)
> 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.
I
I
Ian Eure wrote on 14 Jan 01:48 +0100
[PATCH 1/1] Update text to refer to "tabulation character."
(address . 67118@debbugs.gnu.org)(name . Ian Eure)(address . ian@retrospec.tv)
20240114004833.6152-1-ian@retrospec.tv
Change-Id: I8de95149502e6cb1458e9f4102895395865d7000
---
doc/guix.texi | 2 +-
guix/lint.scm | 4 ++--
tests/lint.scm | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)

Toggle diff (47 lines)
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
?