bug#34419: 27.0.50; [PATCH] Use lexical-binding in conf-mode.el

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#34419: 27.0.50; [PATCH] Use lexical-binding in conf-mode.el

Alex Branham
Hi -

I'd like to apply the following patch to enable lexical binding in
conf-mode.el. OK to apply to master, or should I split the documentation
changes into a separate commit and apply it to emacs-26?

Thanks,
Alex


From 33cd3bc0655dd861a63854bfc0762990da46a7cc Mon Sep 17 00:00:00 2001
From: Alex Branham <[hidden email]>
Date: Sun, 10 Feb 2019 14:32:29 -0600
Subject: [PATCH] * lisp/textmodes/conf-mode.el: Use lexical binding

(conf-align-assignments):
(conf-quote-normal):
(conf-mode-initialize): Fix documentation
---
 lisp/textmodes/conf-mode.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lisp/textmodes/conf-mode.el b/lisp/textmodes/conf-mode.el
index ad79ca4c26..ad9f60fabc 100644
--- a/lisp/textmodes/conf-mode.el
+++ b/lisp/textmodes/conf-mode.el
@@ -1,4 +1,4 @@
-;;; conf-mode.el --- Simple major mode for editing conf/ini/properties files
+;;; conf-mode.el --- Simple major mode for editing conf/ini/properties files  -*- lexical-binding: t; -*-

 ;; Copyright (C) 2004-2019 Free Software Foundation, Inc.

@@ -281,10 +281,10 @@ whitespace.")
 ;; If anybody can figure out how to get the same effect by configuring
 ;; `align', I'd be glad to hear.
 (defun conf-align-assignments (&optional arg)
-  (interactive "P")
   "Align the assignments in the buffer or active region.
 In Transient Mark mode, if the mark is active, operate on the
 contents of the region.  Otherwise, operate on the whole buffer."
+  (interactive "P")
   (setq arg (if arg
  (prefix-numeric-value arg)
       conf-assignment-column))
@@ -323,7 +323,7 @@ contents of the region.  Otherwise, operate on the whole buffer."

 (defun conf-quote-normal (arg)
   "Set the syntax of \\=' and \" to punctuation.
-With prefix arg, only do it for \\=' if 1, or only for \" if 2.
+With prefix ARG, only do it for \\=' if 1, or only for \" if 2.
 This only affects the current buffer.  Some conf files use quotes
 to delimit strings, while others allow quotes as simple parts of
 the assigned value.  In those files font locking will be wrong,
@@ -442,7 +442,7 @@ See also `conf-space-mode', `conf-colon-mode', `conf-javaprop-mode',
     (run-mode-hooks 'conf-mode-hook)))

 (defun conf-mode-initialize (comment &optional font-lock)
-  "Initializations for sub-modes of conf-mode.
+  "Initializations for sub-modes of `conf-mode'.
 COMMENT initializes `comment-start' and `comment-start-skip'.
 The optional arg FONT-LOCK is the value for FONT-LOCK-KEYWORDS."
   (set (make-local-variable 'comment-start) comment)
--
2.19.2



Reply | Threaded
Open this post in threaded view
|

bug#34419: 27.0.50; [PATCH] Use lexical-binding in conf-mode.el

Eli Zaretskii
> From: Alex Branham <[hidden email]>
> Date: Sun, 10 Feb 2019 14:37:26 -0600
>
> I'd like to apply the following patch to enable lexical binding in
> conf-mode.el.

Can you tell how did you test the result to make sure no bugs will be
introduced by lexical-binding in this package?  I see no test suite
for it.

> OK to apply to master, or should I split the documentation
> changes into a separate commit and apply it to emacs-26?

The changes to the documentation are too minor to bother splitting
them, IMO.

> (conf-align-assignments):
> (conf-quote-normal):
> (conf-mode-initialize): Fix documentation

There's no file name in this log message, and it is under-filled (did
you use change-log-mode?).  Also, our style is to say "Doc fix" or
"Docstring fix" in these cases.  Finally, please mention the bug
number in the log message.

Thanks.



Reply | Threaded
Open this post in threaded view
|

bug#34419: 27.0.50; [PATCH] Use lexical-binding in conf-mode.el

Alex Branham

On Mon 11 Feb 2019 at 09:51, Eli Zaretskii <[hidden email]> wrote:

>> From: Alex Branham <[hidden email]>
>> Date: Sun, 10 Feb 2019 14:37:26 -0600
>>
>> I'd like to apply the following patch to enable lexical binding in
>> conf-mode.el.
>
> Can you tell how did you test the result to make sure no bugs will be
> introduced by lexical-binding in this package?  I see no test suite
> for it.

Mostly manual testing. conf-mode isn't that complicated; it's mostly
dealing with font locking. There are a lot of examples in the docstring
though so I've attached a patch that adds some tests based on those. The
tests pass for me both before lexical binding and after. I don't need to
modify the tests/Makefile.in file to have these run, right?

>> OK to apply to master, or should I split the documentation
>> changes into a separate commit and apply it to emacs-26?
>
> The changes to the documentation are too minor to bother splitting
> them, IMO.

That's what I figured. Just wanted to make since
`conf-align-assignments' didn't have documentation before this (well,
not that describe-function would report, anyway).

>> (conf-align-assignments):
>> (conf-quote-normal):
>> (conf-mode-initialize): Fix documentation
>
> There's no file name in this log message, and it is under-filled (did
> you use change-log-mode?).  Also, our style is to say "Doc fix" or
> "Docstring fix" in these cases.  Finally, please mention the bug
> number in the log message.

The file name is in the summary line. Is that not allowed? I was basing
it off of ac1e5a5e2ed7c6cf5bec50e5ebf7fab6792230bd which looks similar.
Either way, the attached patch should be OK I think.


From a2ec79dddbb60f7875b1b492c8e960f6ab331d73 Mon Sep 17 00:00:00 2001
From: Alex Branham <[hidden email]>
Date: Sun, 10 Feb 2019 14:32:29 -0600
Subject: [PATCH 1/2] Use lexical binding for conf-mode

* lisp/textmodes/conf-mode.el: Use lexical binding.
(conf-align-assignments, conf-quote-normal, conf-mode-initialize):
Doc fix.
---
 lisp/textmodes/conf-mode.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lisp/textmodes/conf-mode.el b/lisp/textmodes/conf-mode.el
index ad79ca4c26..ad9f60fabc 100644
--- a/lisp/textmodes/conf-mode.el
+++ b/lisp/textmodes/conf-mode.el
@@ -1,4 +1,4 @@
-;;; conf-mode.el --- Simple major mode for editing conf/ini/properties files
+;;; conf-mode.el --- Simple major mode for editing conf/ini/properties files  -*- lexical-binding: t; -*-
 
 ;; Copyright (C) 2004-2019 Free Software Foundation, Inc.
 
@@ -281,10 +281,10 @@ whitespace.")
 ;; If anybody can figure out how to get the same effect by configuring
 ;; `align', I'd be glad to hear.
 (defun conf-align-assignments (&optional arg)
-  (interactive "P")
   "Align the assignments in the buffer or active region.
 In Transient Mark mode, if the mark is active, operate on the
 contents of the region.  Otherwise, operate on the whole buffer."
+  (interactive "P")
   (setq arg (if arg
  (prefix-numeric-value arg)
       conf-assignment-column))
@@ -323,7 +323,7 @@ contents of the region.  Otherwise, operate on the whole buffer."
 
 (defun conf-quote-normal (arg)
   "Set the syntax of \\=' and \" to punctuation.
-With prefix arg, only do it for \\=' if 1, or only for \" if 2.
+With prefix ARG, only do it for \\=' if 1, or only for \" if 2.
 This only affects the current buffer.  Some conf files use quotes
 to delimit strings, while others allow quotes as simple parts of
 the assigned value.  In those files font locking will be wrong,
@@ -442,7 +442,7 @@ See also `conf-space-mode', `conf-colon-mode', `conf-javaprop-mode',
     (run-mode-hooks 'conf-mode-hook)))
 
 (defun conf-mode-initialize (comment &optional font-lock)
-  "Initializations for sub-modes of conf-mode.
+  "Initializations for sub-modes of `conf-mode'.
 COMMENT initializes `comment-start' and `comment-start-skip'.
 The optional arg FONT-LOCK is the value for FONT-LOCK-KEYWORDS."
   (set (make-local-variable 'comment-start) comment)
--
2.19.2


From d965720495549b92f7a57836a749487ee8201ef5 Mon Sep 17 00:00:00 2001
From: Alex Branham <[hidden email]>
Date: Mon, 11 Feb 2019 16:08:55 -0600
Subject: [PATCH 2/2] Add basic conf-mode tests

* test/lisp/textmodes/conf-mode-tests.el: New file with tests for
  conf-mode.  Mostly taken from conf-mode docstrings.
---
 test/lisp/textmodes/conf-mode-tests.el | 204 +++++++++++++++++++++++++
 1 file changed, 204 insertions(+)
 create mode 100644 test/lisp/textmodes/conf-mode-tests.el

diff --git a/test/lisp/textmodes/conf-mode-tests.el b/test/lisp/textmodes/conf-mode-tests.el
new file mode 100644
index 0000000000..5d79ceec96
--- /dev/null
+++ b/test/lisp/textmodes/conf-mode-tests.el
@@ -0,0 +1,204 @@
+;;; conf-mode-tests.el --- Test suite for conf mode  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2016-2019 Free Software Foundation, Inc.
+
+;; Author: J. Alexander Branham <[hidden email]>
+;; Keywords: internal
+
+;; This file is part of GNU Emacs.
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;; Code:
+(require 'conf-mode)
+(require 'ert)
+
+(ert-deftest conf-test-align-assignments ()
+  "Test for `conf-align-assignments'."
+  (with-temp-buffer
+    (insert "foo: bar\nbar: baz")
+    (conf-colon-mode)
+    (conf-align-assignments)
+    (should (equal (buffer-string)
+                   "foo:                    bar\nbar:                    baz"))))
+
+(ert-deftest conf-test-font-lock ()
+  (with-temp-buffer
+    (insert "foo: bar\nbar: baz")
+    (conf-colon-mode)
+    (font-lock-mode)
+    (font-lock-ensure)
+    (goto-char (point-min))
+    (should (equal (face-at-point) 'font-lock-variable-name-face))
+    (search-forward "bar")
+    (should-not (face-at-point))))
+
+(ert-deftest conf-test-windows-mode ()
+  (with-temp-buffer
+    ;; from `conf-windows-mode' docstring:
+    (insert "[ExtShellFolderViews]
+Default={5984FFE0-28D4-11CF-AE66-08002B2E1262}
+{5984FFE0-28D4-11CF-AE66-08002B2E1262}={5984FFE0-28D4-11CF-AE66-08002B2E1262}
+
+[{5984FFE0-28D4-11CF-AE66-08002B2E1262}]
+PersistMoniker=file://Folder.htt")
+    (goto-char (point-min))
+    (conf-windows-mode)
+    (font-lock-mode)
+    (font-lock-ensure)
+    (search-forward "ExtShell")
+    (should (equal (face-at-point) 'font-lock-type-face))
+    (search-forward "Defau")
+    (should (equal (face-at-point) 'font-lock-variable-name-face))
+    (forward-line)
+    (beginning-of-line)
+    (should (equal (face-at-point) 'font-lock-variable-name-face))
+    (forward-line 2)
+    (should-not (face-at-point))
+    (forward-char)
+    (should (equal (face-at-point) 'font-lock-type-face))))
+
+(ert-deftest conf-test-javaprop-mode ()
+  (with-temp-buffer
+    ;; From `conf-javaprop-mode' docstring
+    (insert "// another kind of comment
+/* yet another */
+
+name:value
+name=value
+name value
+x.1 =
+x.2.y.1.z.1 =
+x.2.y.1.z.2.zz =")
+    (goto-char (point-min))
+    (conf-javaprop-mode)
+    (font-lock-mode)
+    (font-lock-ensure)
+    (should (equal (face-at-point) 'font-lock-comment-delimiter-face))
+    (forward-char 3)
+    (should (equal (face-at-point) 'font-lock-comment-face))
+    (search-forward "*")
+    (should (equal (face-at-point) 'font-lock-comment-delimiter-face))
+    (while (search-forward "nam" nil t)
+      (should (equal (face-at-point) 'font-lock-variable-name-face))
+      (search-forward "val")
+      (should-not (face-at-point)))
+    (while (re-search-forward "a-z" nil t)
+      (backward-char)
+      (should (equal (face-at-point) 'font-lock-variable-name-face))
+      (re-search-forward "[0-0]" nil t)
+      (backward-char)
+      (should (equal (face-at-point) 'font-lock-constant-face)))))
+
+(ert-deftest conf-test-space-mode ()
+  ;; From `conf-space-mode' docstring.
+  (with-temp-buffer
+    (insert "image/jpeg jpeg jpg jpe
+image/png png
+image/tiff tiff tif
+")
+    (goto-char (point-min))
+    (conf-space-mode)
+    (font-lock-mode)
+    (font-lock-ensure)
+    (should (equal (face-at-point) 'font-lock-variable-name-face))
+    (forward-char 15)
+    (should-not (face-at-point))))
+
+(ert-deftest conf-test-colon-mode ()
+  ;; From `conf-colon-mode' docstring.
+  (with-temp-buffer
+    (insert "<Multi_key> <exclam> <exclam> : \"\\241\" exclamdown
+<Multi_key> <c> <slash> : \"\\242\" cent")
+    (goto-char (point-min))
+    (conf-colon-mode)
+    (font-lock-mode)
+    (font-lock-ensure)
+    (should (equal (face-at-point) 'font-lock-variable-name-face))
+    (search-forward "24")
+    (should (equal (face-at-point) 'font-lock-string-face))
+    (forward-line)
+    (should (equal (face-at-point) 'font-lock-variable-name-face))))
+
+(ert-deftest conf-test-ppd-mode ()
+  ;; From `conf-ppd-mode' docstring.
+  (with-temp-buffer
+    (insert "*DefaultTransfer: Null
+*Transfer Null.Inverse: \"{ 1 exch sub }\"")
+    (goto-char (point-min))
+    (conf-ppd-mode)
+    (font-lock-mode)
+    (font-lock-ensure)
+    (should (equal (face-at-point) 'font-lock-variable-name-face))
+    (search-forward "Nul")
+    (should-not (face-at-point))))
+
+(ert-deftest conf-test-xdefaults-mode ()
+  ;; From `conf-xdefaults-mode' docstring.
+  (with-temp-buffer
+    (insert "*background: gray99
+*foreground: black")
+    (goto-char (point-min))
+    (conf-xdefaults-mode)
+    (font-lock-mode)
+    (font-lock-ensure)
+    (should (equal (face-at-point) 'font-lock-variable-name-face))
+    (search-forward "gray")
+    (should-not (face-at-point))))
+
+(ert-deftest conf-test-toml-mode ()
+  ;; From `conf-toml-mode' docstring.
+  (with-temp-buffer
+    (insert "\[entry]
+value = \"some string\"")
+    (goto-char (point-min))
+    (conf-toml-mode)
+    (font-lock-mode)
+    (font-lock-ensure)
+    (should-not (face-at-point))
+    (forward-char)
+    (should (equal (face-at-point) 'font-lock-type-face))
+    (forward-line)
+    (should (equal (face-at-point) 'font-lock-variable-name-face))
+    (search-forward "som")
+    (should (equal (face-at-point) 'font-lock-string-face))))
+
+(ert-deftest conf-test-desktop-mode ()
+  ;; From `conf-desktop-mode' dostring.
+  (with-temp-buffer
+    (insert " [Desktop Entry]
+ Name=GNU Image Manipulation Program
+ Name[oc]=Editor d'imatge GIMP
+ Exec=gimp-2.8 %U
+ Terminal=false")
+    (goto-char (point-min))
+    (conf-desktop-mode)
+    (font-lock-mode)
+    (font-lock-ensure)
+    (search-forward "Desk")
+    (should (equal (face-at-point) 'font-lock-type-face))
+    (search-forward "Nam")
+    (should (equal (face-at-point) 'font-lock-variable-name-face))
+    (forward-char 2)
+    (should-not (face-at-point))
+    (search-forward "[")
+    (should (equal (face-at-point) 'font-lock-constant-face))))
+
+
+
+(provide 'conf-mode-tests)
+
+;;; conf-mode-tests.el ends here
--
2.19.2


signature.asc (497 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#34419: 27.0.50; [PATCH] Use lexical-binding in conf-mode.el

Eli Zaretskii
> From: Alex Branham <[hidden email]>
> Cc: [hidden email]
> Date: Mon, 11 Feb 2019 16:26:57 -0600
>
> > Can you tell how did you test the result to make sure no bugs will be
> > introduced by lexical-binding in this package?  I see no test suite
> > for it.
>
> Mostly manual testing. conf-mode isn't that complicated; it's mostly
> dealing with font locking. There are a lot of examples in the docstring
> though so I've attached a patch that adds some tests based on those. The
> tests pass for me both before lexical binding and after.

Thanks for adding the tests.  One of them fails for me, see the
diagnostics below.

> I don't need to modify the tests/Makefile.in file to have these run,
> right?

Right.

> >> (conf-align-assignments):
> >> (conf-quote-normal):
> >> (conf-mode-initialize): Fix documentation
> >
> > There's no file name in this log message, and it is under-filled (did
> > you use change-log-mode?).  Also, our style is to say "Doc fix" or
> > "Docstring fix" in these cases.  Finally, please mention the bug
> > number in the log message.
>
> The file name is in the summary line. Is that not allowed? I was basing
> it off of ac1e5a5e2ed7c6cf5bec50e5ebf7fab6792230bd which looks similar.

Bad example, IMO.  the problem is that we use the Git log to generate
ChangeLog when we prepare a release tarball, and these deviations make
malformed ChangeLog entries.

> Either way, the attached patch should be OK I think.

Thanks.  As a further nit, please mention the bug number in the log
message when you know it (as you usually do when posting a second
version of the patches).  Otherwise I need to add that manually with
"git commit --amend".

Here's the diagnostics from the failed test; can you spot the problem?

Test conf-test-align-assignments backtrace:
  signal(ert-test-failed (((should (equal (buffer-string) "foo:      
  ert-fail(((should (equal (buffer-string) "foo:                    ba
  (if (unwind-protect (setq value-2 (apply fn-0 args-1)) (setq form-de
  (let (form-description-4) (if (unwind-protect (setq value-2 (apply f
  (let ((value-2 'ert-form-evaluation-aborted-3)) (let (form-descripti
  (let* ((fn-0 #'equal) (args-1 (condition-case err (let ((signal-hook
  (progn (insert "foo: bar\nbar: baz") (conf-colon-mode) (conf-align-a
  (unwind-protect (progn (insert "foo: bar\nbar: baz") (conf-colon-mod
  (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn
  (let ((temp-buffer (generate-new-buffer " *temp*"))) (save-current-b
  (closure (t) nil (let ((temp-buffer (generate-new-buffer " *temp*"))
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name conf-test-align-assignments :document
  ert-run-or-rerun-test(#s(ert--stats :selector (not (tag :unstable))
  ert-run-tests((not (tag :unstable)) #f(compiled-function (event-type
  ert-run-tests-batch((not (tag :unstable)))
  ert-run-tests-batch-and-exit((not (tag :unstable)))
  eval((ert-run-tests-batch-and-exit '(not (tag :unstable))))
  command-line-1((#("-L" 0 2 (charset cp862)) #(";." 0 2 (charset cp86
  command-line()
  normal-top-level()
Test conf-test-align-assignments condition:
    (ert-test-failed
     ((should
       (equal
        (buffer-string)
        "foo:                    bar
bar:                    baz"))
      :form
      (equal "foo: bar
bar: baz" "foo:                    bar
bar:                    baz")
      :value nil :explanation
      (arrays-of-different-length 21 55 "foo: bar
bar: baz" "foo:                    bar
bar:                    baz" first-mismatch-at 4)))
   FAILED   1/10  conf-test-align-assignments (0.000000 sec)



Reply | Threaded
Open this post in threaded view
|

bug#34419: 27.0.50; [PATCH] Use lexical-binding in conf-mode.el

Alex Branham

On Fri 15 Feb 2019 at 02:46, Eli Zaretskii <[hidden email]> wrote:

>> Either way, the attached patch should be OK I think.
>
> Thanks.  As a further nit, please mention the bug number in the log
> message when you know it (as you usually do when posting a second
> version of the patches).  Otherwise I need to add that manually with
> "git commit --amend".

I'm not sure why this is so hard for me to remember, sorry. Will try
harder in the future (including the attached patch!).

> Here's the diagnostics from the failed test; can you spot the problem?

Have you changed `conf-assignment-column'? That would cause issues I
guess. Here's a patch that sets it in the test:

From bbc00e4f70425618f5a6ed59a00afe1d1fcf9427 Mon Sep 17 00:00:00 2001
From: Alex Branham <[hidden email]>
Date: Fri, 15 Feb 2019 07:17:49 -0600
Subject: [PATCH] Fix a conf-mode test

* test/lisp/textmodes/conf-mode-tests.el (conf-test-align-assignments):
  Set `conf-assignment-column'.

Bug#34419
---
 test/lisp/textmodes/conf-mode-tests.el | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/test/lisp/textmodes/conf-mode-tests.el b/test/lisp/textmodes/conf-mode-tests.el
index 5d79ceec96..effe8de397 100644
--- a/test/lisp/textmodes/conf-mode-tests.el
+++ b/test/lisp/textmodes/conf-mode-tests.el
@@ -29,11 +29,12 @@
 (ert-deftest conf-test-align-assignments ()
   "Test for `conf-align-assignments'."
   (with-temp-buffer
-    (insert "foo: bar\nbar: baz")
-    (conf-colon-mode)
-    (conf-align-assignments)
-    (should (equal (buffer-string)
-                   "foo:                    bar\nbar:                    baz"))))
+    (let ((conf-assignment-column 24))
+      (insert "foo: bar\nbar: baz")
+      (conf-colon-mode)
+      (conf-align-assignments)
+      (should (equal (buffer-string)
+                     "foo:                    bar\nbar:                    baz")))))
 
 (ert-deftest conf-test-font-lock ()
   (with-temp-buffer
--
2.19.2






From bbc00e4f70425618f5a6ed59a00afe1d1fcf9427 Mon Sep 17 00:00:00 2001
From: Alex Branham <[hidden email]>
Date: Fri, 15 Feb 2019 07:17:49 -0600
Subject: [PATCH] Fix a conf-mode test

* test/lisp/textmodes/conf-mode-tests.el (conf-test-align-assignments):
  Set `conf-assignment-column'.

Bug#34419
---
 test/lisp/textmodes/conf-mode-tests.el | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/test/lisp/textmodes/conf-mode-tests.el b/test/lisp/textmodes/conf-mode-tests.el
index 5d79ceec96..effe8de397 100644
--- a/test/lisp/textmodes/conf-mode-tests.el
+++ b/test/lisp/textmodes/conf-mode-tests.el
@@ -29,11 +29,12 @@
 (ert-deftest conf-test-align-assignments ()
   "Test for `conf-align-assignments'."
   (with-temp-buffer
-    (insert "foo: bar\nbar: baz")
-    (conf-colon-mode)
-    (conf-align-assignments)
-    (should (equal (buffer-string)
-                   "foo:                    bar\nbar:                    baz"))))
+    (let ((conf-assignment-column 24))
+      (insert "foo: bar\nbar: baz")
+      (conf-colon-mode)
+      (conf-align-assignments)
+      (should (equal (buffer-string)
+                     "foo:                    bar\nbar:                    baz")))))
 
 (ert-deftest conf-test-font-lock ()
   (with-temp-buffer
--
2.19.2


signature.asc (497 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#34419: 27.0.50; [PATCH] Use lexical-binding in conf-mode.el

Eli Zaretskii
> From: Alex Branham <[hidden email]>
> Cc: [hidden email]
> Date: Fri, 15 Feb 2019 07:26:36 -0600
>
> > Thanks.  As a further nit, please mention the bug number in the log
> > message when you know it (as you usually do when posting a second
> > version of the patches).  Otherwise I need to add that manually with
> > "git commit --amend".
>
> I'm not sure why this is so hard for me to remember, sorry. Will try
> harder in the future (including the attached patch!).

No sweat, it will come to you in time.

> > Here's the diagnostics from the failed test; can you spot the problem?
>
> Have you changed `conf-assignment-column'?

No, I haven't.  I run this in batch Emacs, so no local customizations
are in effect.

> Here's a patch that sets it in the test:

Didn't help, same error.  This diagnostic:

      :value nil :explanation
      (arrays-of-different-length 21 55

gives a clue: one of the strings uses TABs to indent, the other uses
spaces.  Maybe bind indent-tabs-mode to the value you want when
running this test.



Reply | Threaded
Open this post in threaded view
|

bug#34419: 27.0.50; [PATCH] Use lexical-binding in conf-mode.el

Alex Branham

On Fri 15 Feb 2019 at 08:10, Eli Zaretskii <[hidden email]> wrote:

>  Maybe bind indent-tabs-mode to the value you want when
> running this test.

Sorry for taking a while to respond; the attached patch should fix the issue.

Thanks,
Alex


From 08027a320721ed52ef92672caf832e089bd7ac25 Mon Sep 17 00:00:00 2001
From: Alex Branham <[hidden email]>
Date: Fri, 15 Feb 2019 07:17:49 -0600
Subject: [PATCH] Fix a conf-mode test

* test/lisp/textmodes/conf-mode-tests.el (conf-test-align-assignments):
  Set `conf-assignment-column' and `indent-tabs-mode'.

Bug#34419
---
 test/lisp/textmodes/conf-mode-tests.el | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/test/lisp/textmodes/conf-mode-tests.el b/test/lisp/textmodes/conf-mode-tests.el
index 5d79ceec96..33009215bb 100644
--- a/test/lisp/textmodes/conf-mode-tests.el
+++ b/test/lisp/textmodes/conf-mode-tests.el
@@ -29,11 +29,13 @@
 (ert-deftest conf-test-align-assignments ()
   "Test for `conf-align-assignments'."
   (with-temp-buffer
-    (insert "foo: bar\nbar: baz")
-    (conf-colon-mode)
-    (conf-align-assignments)
-    (should (equal (buffer-string)
-                   "foo:                    bar\nbar:                    baz"))))
+    (let ((conf-assignment-column 24)
+          indent-tabs-mode)
+      (insert "foo: bar\nbar: baz")
+      (conf-colon-mode)
+      (conf-align-assignments)
+      (should (equal (buffer-string)
+                     "foo:                    bar\nbar:                    baz")))))
 
 (ert-deftest conf-test-font-lock ()
   (with-temp-buffer
--
2.19.2

Reply | Threaded
Open this post in threaded view
|

bug#34419: 27.0.50; [PATCH] Use lexical-binding in conf-mode.el

Eli Zaretskii
> From: Alex Branham <[hidden email]>
> Cc: [hidden email]
> Date: Mon, 18 Feb 2019 11:17:30 -0600
>
> >  Maybe bind indent-tabs-mode to the value you want when
> > running this test.
>
> Sorry for taking a while to respond; the attached patch should fix the issue.

Thanks, this fixes the failure.



Reply | Threaded
Open this post in threaded view
|

bug#34419: 27.0.50; [PATCH] Use lexical-binding in conf-mode.el

Alex Branham
On Mon 18 Feb 2019 at 11:44, Eli Zaretskii <[hidden email]> wrote:

> Thanks, this fixes the failure.

Thanks, I'll close the bug report then.

Alex

signature.asc (497 bytes) Download Attachment