bug#19709: Problem with indentation for the closing parenthesis in cperl-mode version 6.1

bug#19709: Problem with indentation for the closing parenthesis in cperl-mode version 6.1

[Håkon Hægland]

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

I have problems with correct alignment of the closing parenthesis in
cperl-mode. For example consider the following Perl code:

my $a = func1(
    Module::test()
  );

my $b = func2(
    test()
);

my $c = func3(
    Module::test(),
);
We see that the closing parenthesis is not correctly indented for func1,
but for func2 and func3 it is correct.

In cperl-mode-hook I have

(setq cperl-indent-level 4)
(setq cperl-indent-parens-as-block t)
(setq cperl-close-paren-offset -4)

It seems to me that this behavior is related to the newest version of
cperl-mode. I am using Emacs 24.4 on Ubuntu 14.04, and the cperl-mode that
came with the installation
(/opt/emacs24.4/share/emacs/24.4/lisp/progmodes/cperl-mode.el.gz) is
version 6.1 of cperl-mode.

I then tried to install cperl-mode from ELPA using Emacs 24.4 package
manager (list-packages), and it turned out that it gave me version 5.23 of
cperl-mode which also indented the closing parenthesis correctly..

M-x emacs-version:

GNU Emacs 24.4.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.8) of
2015-01-11 on hakon-ThinkPad-Edge-E540

I built Emacs 24.4.1 from source, using with `--with-xft` option to
configure.

Best regards,
Håkon Hægland

[-- Attachment #2: Type: text/html, Size: 1635 bytes --]

bug#19709: Problem with indentation for the closing parenthesis in cperl-mode version 6.1

[Stefan Kangas]

Håkon Hægland <hakon.hagland@gmail.com> writes:

> I have problems with correct alignment of the closing parenthesis in cperl-mode. For example consider the following Perl code:
>
> my $a = func1(
>     Module::test()
>   );
>
> my $b = func2(
>     test()
> );
>
> my $c = func3(
>     Module::test(),
> );
> We see that the closing parenthesis is not correctly indented for func1, but for func2 and func3 it is correct.
>
> In cperl-mode-hook I have
>
> (setq cperl-indent-level 4)
> (setq cperl-indent-parens-as-block t)
> (setq cperl-close-paren-offset -4)
>
> It seems to me that this behavior is related to the newest version of cperl-mode. I am using Emacs 24.4 on Ubuntu 14.04, and the cperl-mode that came with the installation (/opt/emacs24.4/share/emacs/24.4/lisp/progmodes/cperl-mode.el.gz) is version 6.1
> of cperl-mode.
>
> I then tried to install cperl-mode from ELPA using Emacs 24.4 package manager (list-packages), and it turned out that it gave me version 5.23 of cperl-mode which also indented the closing parenthesis correctly..

I can reproduce this on Emacs 26.2.

BTW, the problem goes away when I set:

  (setq cperl-label-offset 0)

But of course you shouldn't have to do that.

Best regards,
Stefan Kangas

bug#19709: [PATCH] Verify that indentation works with current cperl-mode

[Harald Jörg]

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

This bug apparently can no longer be reproduced.

Using quick'n'dirty bisection (avoiding recompilations of the whole
source tree) I could track the fix back to commit 1bcbcb7e,
"lisp/progmodes/cperl-mode.el: Merge from Jonathan Rockway's version" by
Stefan Monnier, 21st Dec 2017.  This makes sense since this commit
introduced a new function 'cperl-calculate-indent' to cperl-mode.el.

The patch adds a test to verify correct behavior, and also adds the
relevant variable cperl-close-paren-offset (which is explicitly set in
the bug report) to the scope of the cperl-set-style setting "PBP".  This
variable is documented in the book, but wasn't included yet.
--
Cheers,
haj


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Indentation obeys customisation --]
[-- Type: text/x-diff, Size: 5034 bytes --]

From 91761ee3e83cfad9ccabdd75dd364e9bc84db145 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Harald=20J=C3=B6rg?= <haj@posteo.de>
Date: Wed, 4 Nov 2020 18:39:24 +0100
Subject: [PATCH] ; cperl-mode: Indentation of ')' follows customisation

* test/lisp/progmodes/cperl-mode-tests.el (cperl-bug19709):
New test to verify correct indentation of closing parentheses (Bug#19709).

* test/lisp/progmodes/cperl-mode-resources/cperl-bug-19709.pl:
New test case with code from the bug report.

* lisp/progmodes/cperl-mode.el (cperl-style-alist): Add
cperl-close-paren-offset to the settings for PBP style.

* test/lisp/progmodes/cperl-mode-resources/cperl-indent-styles.pl:
Add a new test clause for cperl-close-paren-offset.
---
 lisp/progmodes/cperl-mode.el                  |  1 +
 .../cperl-mode-resources/cperl-bug-19709.pl   | 25 ++++++++++++
 .../cperl-indent-styles.pl                    | 10 +++++
 test/lisp/progmodes/cperl-mode-tests.el       | 38 +++++++++++++++++++
 4 files changed, 74 insertions(+)
 create mode 100644 test/lisp/progmodes/cperl-mode-resources/cperl-bug-19709.pl

diff --git a/lisp/progmodes/cperl-mode.el b/lisp/progmodes/cperl-mode.el
index 6178cdfc9b..d5b30025e7 100644
--- a/lisp/progmodes/cperl-mode.el
+++ b/lisp/progmodes/cperl-mode.el
@@ -5983,6 +5983,7 @@ cperl-style-alist
      (cperl-continued-brace-offset     .  0)
      (cperl-label-offset               . -2)
      (cperl-continued-statement-offset .  4)
+     (cperl-close-paren-offset         . -4)
      (cperl-extra-newline-before-brace .  nil)
      (cperl-extra-newline-before-brace-multiline .  nil)
      (cperl-merge-trailing-else        .  nil)
diff --git a/test/lisp/progmodes/cperl-mode-resources/cperl-bug-19709.pl b/test/lisp/progmodes/cperl-mode-resources/cperl-bug-19709.pl
new file mode 100644
index 0000000000..f7c51a2ce5
--- /dev/null
+++ b/test/lisp/progmodes/cperl-mode-resources/cperl-bug-19709.pl
@@ -0,0 +1,25 @@
+# -------- bug#19709: input --------
+my $a = func1(
+    Module::test()
+  );
+
+my $b = func2(
+    test()
+);
+
+my $c = func3(
+    Module::test(),
+);
+# -------- bug#19709: expected output --------
+my $a = func1(
+    Module::test()
+);
+
+my $b = func2(
+    test()
+);
+
+my $c = func3(
+    Module::test(),
+);
+# -------- bug#19709: end --------
diff --git a/test/lisp/progmodes/cperl-mode-resources/cperl-indent-styles.pl b/test/lisp/progmodes/cperl-mode-resources/cperl-indent-styles.pl
index 0832f86828..371b19b730 100644
--- a/test/lisp/progmodes/cperl-mode-resources/cperl-indent-styles.pl
+++ b/test/lisp/progmodes/cperl-mode-resources/cperl-indent-styles.pl
@@ -42,3 +42,13 @@
     }
 }
 # -------- PBP uncuddle else: end --------
+
+# -------- PBP closing paren offset: input --------
+my $a = func1(
+    Module::test()
+  );
+# -------- PBP closing paren offset: expected output --------
+my $a = func1(
+    Module::test()
+);
+# -------- PBP closing paren offset: end --------
diff --git a/test/lisp/progmodes/cperl-mode-tests.el b/test/lisp/progmodes/cperl-mode-tests.el
index 2977f10813..a262db3136 100644
--- a/test/lisp/progmodes/cperl-mode-tests.el
+++ b/test/lisp/progmodes/cperl-mode-tests.el
@@ -249,6 +249,44 @@ cperl-bug30393
             (setq got (concat "test case " name ":\n" (buffer-string)))
             (should (equal got expected))))))))
 
+(ert-deftest cperl-bug19709 ()
+  "Verify that indentation of closing paren works as intended.
+Note that Perl mode has no setting for close paren offset, per
+documentation it does the right thing anyway."
+  (let ((file (ert-resource-file "cperl-bug-19709.pl")))
+    (with-temp-buffer
+      (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)
+	    (funcall cperl-test-mode)
+            (setq-local
+             ;; settings from the bug report
+             cperl-indent-level 4
+             cperl-indent-parens-as-block t
+             cperl-close-paren-offset -4
+             ;; same, adapted for per-mode
+             perl-indent-level 4
+             perl-indent-parens-as-block t)
+            (goto-char (point-min))
+            (while (null (eobp))
+              (cperl-indent-command)
+              (next-line))
+            (setq expected (concat "test case " name ":\n" expected))
+            (setq got (concat "test case " name ":\n" (buffer-string)))
+            (should (equal got expected))))))))
+
 (ert-deftest cperl-bug37127 ()
   "Verify that closing a paren in a regex goes without a message.
 Also check that the message is issued if the regex terminator is
-- 
2.20.1

bug#19709: [PATCH] Verify that indentation works with current cperl-mode

[Lars Ingebrigtsen]

haj@posteo.de (Harald Jörg) writes:

> The patch adds a test to verify correct behavior, and also adds the
> relevant variable cperl-close-paren-offset (which is explicitly set in
> the bug report) to the scope of the cperl-set-style setting "PBP".  This
> variable is documented in the book, but wasn't included yet.

Thanks; applied to Emacs 28.  One note:

> +            (while (null (eobp))
> +              (cperl-indent-command)
> +              (next-line))

Is this on purpose to test `next-line'?   If it is, this warning should
probably be inhibited:

lisp/progmodes/cperl-mode-tests.el:284:16: Warning: ‘next-line’ is for
    interactive use only; use ‘forward-line’ instead.

If not, forward-line should be used.

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

bug#19709: [PATCH] Verify that indentation works with current cperl-mode

[Harald Jörg]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> [...]
> Thanks; applied to Emacs 28.  One note:
>
>> +            (while (null (eobp))
>> +              (cperl-indent-command)
>> +              (next-line))
>
> Is this on purpose to test `next-line'?   If it is, this warning should
> probably be inhibited:
> 
> lisp/progmodes/cperl-mode-tests.el:284:16: Warning: ‘next-line’ is for
>     interactive use only; use ‘forward-line’ instead.

Nah - that's still me fighting with Emacs Lisp vocabulary.  Sorry for
that.  I'll replace it with (forward-line 1).

I should submit a patch anyway since I've found that the usage of
setq-local for a list of variables isn't available in Emacs 26, and I
want to run the tests in that version, too.

--
Thanks for reminding me,
haj

bug#19709: [PATCH] Verify that indentation works with current cperl-mode

[Lars Ingebrigtsen]

haj@posteo.de (Harald Jörg) writes:

> Nah - that's still me fighting with Emacs Lisp vocabulary.  Sorry for
> that.  I'll replace it with (forward-line 1).
>
> I should submit a patch anyway since I've found that the usage of
> setq-local for a list of variables isn't available in Emacs 26, and I
> want to run the tests in that version, too.

Sounds good.

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