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 <eliz@gnu.org> wrote:

>> From: Alex Branham <alex.branham@gmail.com>
>> 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 <alex.branham@gmail.com>
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 <alex.branham@gmail.com>
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 <alex.branham@gmail.com>
+;; 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
Description: PGP signature