Fwd: Requesting review for change to lisp/textmodes/sgml-mode.el

[Jackson Hamilton <jackson@jacksonrayhamilton.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1404 bytes --]

Hey guys, still hoping to get this reviewed. I wouldn't want to merge
something in that wasn't given the "A-OK."

---------- Forwarded message ----------
From: Jackson Hamilton <jackson@jacksonrayhamilton.com>
Date: Wed, Feb 25, 2015 at 2:35 AM
Subject: Requesting review for change to lisp/textmodes/sgml-mode.el
To: emacs-devel <emacs-devel@gnu.org>


Hello comrades,

I made an adjustment (fix?) to the way SGML attributes are indented.

Previously, if one wrote a form like the following:

<element attribute="value">

He could break the attribute onto a new line and it would be indented like
so:

<element
   attribute="value">

But sgml-basic-offset defaults to 2, not 3, so it doesn't make much sense
that
the attribute is indented by 3 spaces.

And if I (setq sgml-basic-offset 4), now my attributes are indented by 5
spaces. Personally I do not expect this behavior, I expect the indentation
to
match.

Perhaps it could be argued that the extra space helps to improve
readability;
maybe so, but it still seems to contradict the offset value. In teams where
many
people use editors that insert multiples of N spaces or tabs, this +1
indentation strategy feels rather alienating. I think it would be better to
stick to a multiple of the specified offset when an attribute is sitting on
its
own line.

Hence the attached patch to remove the +1 indentation behavior.

Thanks for reviewing,
Jackson

[-- Attachment #1.2: Type: text/html, Size: 2093 bytes --]

[-- Attachment #2: 0001-lisp-textmodes-sgml-mode.el-sgml-calculate-indent-Fi.patch --]
[-- Type: text/x-patch, Size: 1560 bytes --]

From 4e4f4519641946ee0b36836646fb339cbafad182 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Wed, 25 Feb 2015 01:56:49 -0800
Subject: [PATCH] * lisp/textmodes/sgml-mode.el (sgml-calculate-indent): Fix
 indent.

Previously, SGML attributes were always indented one additional space
past `sgml-basic-offset'. This fixes that.
---
 lisp/ChangeLog              | 5 +++++
 lisp/textmodes/sgml-mode.el | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index fc2893e..17717e0 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,8 @@
+2015-02-25  Jackson Ray Hamilton  <jackson@jacksonrayhamilton.com>
+
+	* lisp/textmodes/sgml-mode.el (sgml-calculate-indent): Fix
+	attribute indentation.
+
 2015-02-25  Stefan Monnier  <monnier@iro.umontreal.ca>
 
 	* emacs-lisp/edebug.el (edebug--display): Save-excursion (bug#19611).
diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index 12d98c8..887c70d 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -1510,13 +1510,13 @@ LCON is the lexical context, if any."
     (`pi nil)
 
     (`tag
-     (goto-char (1+ (cdr lcon)))
+     (goto-char (cdr lcon))
      (skip-chars-forward "^ \t\n")	;Skip tag name.
      (skip-chars-forward " \t")
      (if (not (eolp))
 	 (current-column)
        ;; This is the first attribute: indent.
-       (goto-char (1+ (cdr lcon)))
+       (goto-char (cdr lcon))
        (+ (current-column) sgml-basic-offset)))
 
     (`text
-- 
1.9.1