bug#43457: 28.0.50; [FEATURE][PATCH] cperl-mode: Add value "PBP" to cperl-set-style

bug#43457: 28.0.50; [FEATURE][PATCH] cperl-mode: Add value "PBP" to cperl-set-style

[Harald Jörg]

[-- Attachment #1: Type: text/plain, Size: 999 bytes --]

This is not a bug report, but a feature request.  If I understand
correctly, both go through the same channel.

Motivation: cperl-mode offers a command 'cperl-set-style' which sets
some customization values for code formatting according to a list of
rule sets.  The book "Perl Best Practices", known as "PBP" in the Perl
community, also defines such a rule set, including the corresponding
settings for Emacs (among other editors).

The PBP rules are widely accepted for Perl code, so in my opinion it
makes sense to make these settings available through 'cperl-set-style'.

The attached patch adds the new value "PBP" to the style selection and
to the documentation, and also contains a simple formatting test
according to these rules.

I also added some lines to etc/NEWS, but I'm unsure about the handling
of "---" to indicate that the change is not relevant to the manual:
There is no texinfo manual for CPerl mode, so maybe "---" should be
added before the heading "CPerl Mode"?
--
Cheers,
haj

[-- Attachment #2: 0001-cperl-mode-Add-new-value-PBP-for-cperl-set-style.patch --]
[-- Type: text/x-patch, Size: 7765 bytes --]

From 14d2c238eafbaf64c29b845cb4de9e4bbac7217f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Harald=20J=C3=B6rg?= <haj@posteo.de>
Date: Wed, 16 Sep 2020 22:30:01 +0200
Subject: [PATCH] cperl-mode: Add new value "PBP" for 'cperl-set-style'

* lisp/progmodes/cperl-mode.el (cperl-style-alist,
cperl-set-style): Add indentation style recommended by Damian Conway's
book "Perl Best Practices".

* test/lisp/progmodes/cperl-mode-tests.el (cperl-mode-test-indent-styles):
Add a test to verify indentation and unraveling of conditionals.
---
 etc/NEWS                                      |  5 +++
 lisp/progmodes/cperl-mode.el                  | 41 +++++++++++++----
 .../cperl-indent-styles.pl                    | 44 +++++++++++++++++++
 test/lisp/progmodes/cperl-mode-tests.el       | 32 ++++++++++++++
 4 files changed, 114 insertions(+), 8 deletions(-)
 create mode 100644 test/lisp/progmodes/cperl-mode-resources/cperl-indent-styles.pl

diff --git a/etc/NEWS b/etc/NEWS
index 81a4273b0f..8b336a8aa9 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1168,6 +1168,11 @@ non-nil, even if protected by 'dbus-ignore-errors' otherwise.
 
 ---
 *** D-Bus events keep the type information of their arguments.
+** CPerl Mode
+*** The command 'cperl-set-style' offers the new value "PBP".
+This value customizes Emacs to use the style recommended in Damian
+Conway's book "Perl Best Practices" for indentation and formatting
+of conditionals.
 
 \f
 * New Modes and Packages in Emacs 28.1
diff --git a/lisp/progmodes/cperl-mode.el b/lisp/progmodes/cperl-mode.el
index af179e2797..8804e83fce 100644
--- a/lisp/progmodes/cperl-mode.el
+++ b/lisp/progmodes/cperl-mode.el
@@ -1234,6 +1234,7 @@ cperl-old-style
 	  ["Auto fill" auto-fill-mode t])
 	 ("Indent styles..."
 	  ["CPerl" (cperl-set-style "CPerl") t]
+	  ["PBP" (cperl-set-style  "PBP") t]
 	  ["PerlStyle" (cperl-set-style "PerlStyle") t]
 	  ["GNU" (cperl-set-style "GNU") t]
 	  ["C++" (cperl-set-style "C++") t]
@@ -1553,12 +1554,12 @@ cperl-mode
  `cperl-min-label-indent'
     Minimal indentation for line that is a label.
 
-Settings for classic indent-styles: K&R BSD=C++ GNU PerlStyle=Whitesmith
-  `cperl-indent-level'                5   4       2   4
-  `cperl-brace-offset'                0   0       0   0
-  `cperl-continued-brace-offset'     -5  -4       0   0
-  `cperl-label-offset'               -5  -4      -2  -4
-  `cperl-continued-statement-offset'  5   4       2   4
+Settings for classic indent-styles: K&R BSD=C++ GNU PBP PerlStyle=Whitesmith
+  `cperl-indent-level'                5   4       2   4   4
+  `cperl-brace-offset'                0   0       0   0   0
+  `cperl-continued-brace-offset'     -5  -4       0   0   0
+  `cperl-label-offset'               -5  -4      -2  -2  -4
+  `cperl-continued-statement-offset'  5   4       2   4   4
 
 CPerl knows several indentation styles, and may bulk set the
 corresponding variables.  Use \\[cperl-set-style] to do this.  Use
@@ -6046,7 +6047,19 @@ cperl-style-examples
   stop;
 }
 
-### PerlStyle	(=CPerl with 4 as indent)		4/0/0/-4/4/t/nil
+### PBP (=Perl Best Practices)				4/0/0/-4/4/nil/nil
+if (foo) {
+    bar
+	baz;
+  label:
+    {
+	boon;
+    }
+}
+else {
+    stop;
+}
+### PerlStyle	(=CPerl with 4 as indent)		4/0/0/-2/4/t/nil
 if (foo) {
     bar
 	baz;
@@ -6149,6 +6162,18 @@ cperl-style-alist
      (cperl-extra-newline-before-brace-multiline .  nil)
      (cperl-merge-trailing-else	       .  t))
 
+    ("PBP"  ;; Perl Best Practices by Damian Conway
+     (cperl-indent-level               .  4)
+     (cperl-brace-offset               .  0)
+     (cperl-continued-brace-offset     .  0)
+     (cperl-label-offset               . -2)
+     (cperl-continued-statement-offset .  4)
+     (cperl-extra-newline-before-brace .  nil)
+     (cperl-extra-newline-before-brace-multiline .  nil)
+     (cperl-merge-trailing-else        .  nil)
+     (cperl-indent-parens-as-block     .  t)
+     (cperl-tab-always-indent          .  t))
+
     ("PerlStyle"			; CPerl with 4 as indent
      (cperl-indent-level               .  4)
      (cperl-brace-offset               .  0)
@@ -6220,7 +6245,7 @@ cperl-set-style
   "Set CPerl mode variables to use one of several different indentation styles.
 The arguments are a string representing the desired style.
 The list of styles is in `cperl-style-alist', available styles
-are CPerl, PerlStyle, GNU, K&R, BSD, C++ and Whitesmith.
+are CPerl, PBP, PerlStyle, GNU, K&R, BSD, C++ and Whitesmith.
 
 The current value of style is memorized (unless there is a memorized
 data already), may be restored by `cperl-set-style-back'.
diff --git a/test/lisp/progmodes/cperl-mode-resources/cperl-indent-styles.pl b/test/lisp/progmodes/cperl-mode-resources/cperl-indent-styles.pl
new file mode 100644
index 0000000000..0832f86828
--- /dev/null
+++ b/test/lisp/progmodes/cperl-mode-resources/cperl-indent-styles.pl
@@ -0,0 +1,44 @@
+#!/usr/bin/env perl
+use strict;
+use warnings;
+use 5.020;
+
+# This file contains test input and expected output for the tests in
+# cperl-mode-tests.el, cperl-mode-test-indent-exp.  The code is
+# syntactically valid, but doesn't make much sense.
+
+# -------- PBP indent: input --------
+for my $foo (@ARGV)
+{
+...;
+}
+# -------- PBP indent: expected output --------
+for my $foo (@ARGV) {
+    ...;
+}
+# -------- PBP indent: end --------
+
+# -------- PBP uncuddle else: input --------
+{
+if (1 < 2)
+{
+say "Seems ok";
+} elsif (1 == 2) {
+say "Strange things are happening";
+} else {
+die "This world is backwards";
+}
+}
+# -------- PBP uncuddle else: expected output --------
+{
+    if (1 < 2) {
+	say "Seems ok";
+    }
+    elsif (1 == 2) {
+	say "Strange things are happening";
+    }
+    else {
+	die "This world is backwards";
+    }
+}
+# -------- PBP uncuddle else: end --------
diff --git a/test/lisp/progmodes/cperl-mode-tests.el b/test/lisp/progmodes/cperl-mode-tests.el
index 2eaf633d17..8e3b73d649 100644
--- a/test/lisp/progmodes/cperl-mode-tests.el
+++ b/test/lisp/progmodes/cperl-mode-tests.el
@@ -172,4 +172,35 @@ cperl-mode-test-indent-exp
             (setq got (concat "test case " name ":\n" (buffer-string)))
             (should (equal got expected))))))))
 
+(ert-deftest cperl-mode-test-indent-styles ()
+  "Verify correct indentation by style \"PBP\".
+Perl Best Practices sets some indentation values different from
+  the defaults, and also wants an \"else\" or \"elsif\" keyword
+  to align with the \"if\"."
+  (let ((file (expand-file-name "cperl-indent-styles.pl"
+                                cperl-mode-tests-data-directory)))
+    (with-temp-buffer
+      (cperl-set-style "PBP")
+      (insert-file-contents file)
+      (goto-char (point-min))
+      (while (re-search-forward
+              (concat "^# ?-+ \\_<\\(?1:.+?\\)\\_>: input ?-+\n"
+                      "\\(?2:\\(?:.*\n\\)+?\\)"
+                      "# ?-+ \\1: expected output ?-+\n"
+                      "\\(?3:\\(?:.*\n\\)+?\\)"
+                      "# ?-+ \\1: end ?-+")
+              nil t)
+        (let ((name (match-string 1))
+              (code (match-string 2))
+              (expected (match-string 3))
+              got)
+          (with-temp-buffer
+            (insert code)
+	    (cperl-mode)
+	    (indent-region (point-min) (point-max)) ; here we go!
+            (setq expected (concat "test case " name ":\n" expected))
+            (setq got (concat "test case " name ":\n" (buffer-string)))
+            (should (equal got expected)))))
+      (cperl-set-style "CPerl"))))
+
 ;;; cperl-mode-tests.el ends here
-- 
2.20.1

bug#43457: 28.0.50; [FEATURE][PATCH] cperl-mode: Add value "PBP" to cperl-set-style

[Lars Ingebrigtsen]

Harald Jörg <haj@posteo.de> writes:

> This is not a bug report, but a feature request.  If I understand
> correctly, both go through the same channel.

Yup.

> The attached patch adds the new value "PBP" to the style selection and
> to the documentation, and also contains a simple formatting test
> according to these rules.

Looks great!  Applied to Emacs 28.

> I also added some lines to etc/NEWS, but I'm unsure about the handling
> of "---" to indicate that the change is not relevant to the manual:
> There is no texinfo manual for CPerl mode, so maybe "---" should be
> added before the heading "CPerl Mode"?

Yup; I did so before committing.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no