some proposed tweaks to HTML mode

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

some proposed tweaks to HTML mode

Eric Abrahamsen-2
I've always found vanilla HTML mode kind of annoying to use, and finally
tried to figure out why. (I assume everyone is using some fancier HTML
authoring mode, but that's no reason not to fix this one.)

Embarrassingly, the main pain point seemed to be that, when you insert
<ul> or <ol>, the closing tag isn't indented properly. Not the end of
the world, but enough to bug me. That led to a few other tweaks, and I'm
inserting the whole diff here for consideration, with notes:

diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index 21b7082b85..9aae47ca09 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -775,8 +775,9 @@ sgml-attributes
  (symbolp (car (car alist))))
     (setq car (car alist)
   alist (cdr alist)))
- (or quiet
-    (message "No attributes configured."))
+        (unless (or alist quiet)
+  (message "No attributes configured.")))

Currently, this function _always_ flashes the "No attributes
configured" message.

(if (stringp (car alist))
     (progn
       (insert (if (eq (preceding-char) ?\s) "" ?\s)
@@ -1743,6 +1744,8 @@ html-mode-map
     (define-key map "\C-c1" 'html-headline-1)
     (define-key map "\C-c\r" 'html-paragraph)
     (define-key map "\C-c\n" 'html-line)
+    (define-key map "\C-cd" 'html-div)
+    (define-key map "\C-cs" 'html-span)
     (define-key map "\C-c\C-c-" 'html-horizontal-rule)
     (define-key map "\C-c\C-co" 'html-ordered-list)
     (define-key map "\C-c\C-cu" 'html-unordered-list)
@@ -1962,7 +1965,7 @@ html-tag-alist
       ("dd" ,(not sgml-xml-mode))
       ("del" nil ("cite") ("datetime"))
       ("dfn")
-      ("div")
+      ("div" nil ("class") ("id"))

The <div> and <span> elements have literally no use except for a place
to hang the "class" and/or "id" attribute, so I don't see why we don't
accept those here. In fact, I'd rather just automatically add "class"
and "id" as potential attributes on all the tags.

       ("dl" (nil \n
  ( "Term: "
    "<dt>" str (if sgml-xml-mode "</dt>")
@@ -2045,7 +2048,8 @@ html-tag-alist
  ("string")
  ("type")
  ("variable-name")
- ("warning")))
+ ("warning"))
+        ("id"))
       ("strong")
       ("style" \n ("type") ("media") ("title"))
       ("sub")
@@ -2451,26 +2455,38 @@ html-ordered-list
   nil
   "<ol>" \n
   "<li>" _ (if sgml-xml-mode "</li>") \n
-  "</ol>")
+  "</ol>" >)
 
 (define-skeleton html-unordered-list
   "HTML unordered list tags."
   nil
   "<ul>" \n
   "<li>" _ (if sgml-xml-mode "</li>") \n
-  "</ul>")
+  "</ul>" >)
 
 (define-skeleton html-list-item
   "HTML list item tag."
   nil
-  (if (bolp) nil '\n)
+  (if (re-search-backward "^[[:blank:]]+") nil '\n)
   "<li>" _ (if sgml-xml-mode "</li>"))
 
 (define-skeleton html-paragraph
   "HTML paragraph tag."
   nil
-  (if (bolp) nil ?\n)
-  "<p>" _ (if sgml-xml-mode "</p>"))
+  (if (re-search-backward "^[[:blank:]]+") nil ?\n)
+  "<p>" > _ (if sgml-xml-mode "</p>"))
+
+(define-skeleton html-div
+  "HTML div tag."
+  nil
+  (if (re-search-backward "^[[:blank:]]+") nil ?\n)
+  "<div>" > \n _ \n "</div>" >)
+
+(define-skeleton html-span
+  "HTML span tag."
+  nil
+  (if (re-search-backward "^[[:blank:]]+") nil ?\n)
+  "<span>" > _ "</span>")
 
 (define-skeleton html-checkboxes
   "Group of connected checkbox inputs."


The above changes (in addition to adding skeletons for div and span
tags) assume that what we want is a nicely indented HTML buffer. Nested
divs should be indented, etc. This is what I want when writing HTML, but
it's an assumption that might not be welcomed by everyone. Also, there
might be a better way of ensuring proper indentation, I'm not that
familiar with skeletons. Essentially, in my opinion, every line should
always be indented.

WDYT?
Eric


Reply | Threaded
Open this post in threaded view
|

Re: some proposed tweaks to HTML mode

Andreas Schwab-2
On Mär 21 2019, Eric Abrahamsen <[hidden email]> wrote:

> @@ -1743,6 +1744,8 @@ html-mode-map
>      (define-key map "\C-c1" 'html-headline-1)
>      (define-key map "\C-c\r" 'html-paragraph)
>      (define-key map "\C-c\n" 'html-line)
> +    (define-key map "\C-cd" 'html-div)
> +    (define-key map "\C-cs" 'html-span)

C-c <letter> is reserved for the user, and no mode should bind them.

Andreas.

--
Andreas Schwab, [hidden email]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Reply | Threaded
Open this post in threaded view
|

Re: some proposed tweaks to HTML mode

Eric Abrahamsen-2
Andreas Schwab <[hidden email]> writes:

> On Mär 21 2019, Eric Abrahamsen <[hidden email]> wrote:
>
>> @@ -1743,6 +1744,8 @@ html-mode-map
>>      (define-key map "\C-c1" 'html-headline-1)
>>      (define-key map "\C-c\r" 'html-paragraph)
>>      (define-key map "\C-c\n" 'html-line)
>> +    (define-key map "\C-cd" 'html-div)
>> +    (define-key map "\C-cs" 'html-span)
>
> C-c <letter> is reserved for the user, and no mode should bind them.

Ah, thanks for that catch. And I see that's exactly what
`html-quick-keys' is for. I've regularized the keybindings accordingly.

Thanks,
Eric


Reply | Threaded
Open this post in threaded view
|

Re: some proposed tweaks to HTML mode

Eric Abrahamsen-2
Eric Abrahamsen <[hidden email]> writes:

> Andreas Schwab <[hidden email]> writes:
>
>> On Mär 21 2019, Eric Abrahamsen <[hidden email]> wrote:
>>
>>> @@ -1743,6 +1744,8 @@ html-mode-map
>>>      (define-key map "\C-c1" 'html-headline-1)
>>>      (define-key map "\C-c\r" 'html-paragraph)
>>>      (define-key map "\C-c\n" 'html-line)
>>> +    (define-key map "\C-cd" 'html-div)
>>> +    (define-key map "\C-cs" 'html-span)
>>
>> C-c <letter> is reserved for the user, and no mode should bind them.
>
> Ah, thanks for that catch. And I see that's exactly what
> `html-quick-keys' is for. I've regularized the keybindings accordingly.

I also find this part of `sgml-attributes' really weird. `alist' is the
list of valid attributes. It counts down the number of valid attributes,
but doesn't actually keep track of which you've added: the <img> tag has
nine distinct valid attributes, but the code will happily let you add
nine "src" attributes and then bail. It also doesn't handle
keyboard-quit gracefully. Ideally this would pop attributes off the list
of acceptable choices, and also delete extra spaces on keyboard quit.

(setq i (length alist))
(while (> i 0)
  (insert ?\s)
  (insert (funcall skeleton-transformation-function
                   (setq attribute
                         (skeleton-read (lambda ()
                                          (completing-read
                                           "Attribute: "
                                           alist))))))
  (if (string= "" attribute)
      (setq i 0)
    (sgml-value (assoc (downcase attribute) alist))
    (setq i (1- i))))


Reply | Threaded
Open this post in threaded view
|

Re: some proposed tweaks to HTML mode

Eric Abrahamsen-2
In reply to this post by Eric Abrahamsen-2
Eric Abrahamsen <[hidden email]> writes:

> Andreas Schwab <[hidden email]> writes:
>
>> On Mär 21 2019, Eric Abrahamsen <[hidden email]> wrote:
>>
>>> @@ -1743,6 +1744,8 @@ html-mode-map
>>>      (define-key map "\C-c1" 'html-headline-1)
>>>      (define-key map "\C-c\r" 'html-paragraph)
>>>      (define-key map "\C-c\n" 'html-line)
>>> +    (define-key map "\C-cd" 'html-div)
>>> +    (define-key map "\C-cs" 'html-span)
>>
>> C-c <letter> is reserved for the user, and no mode should bind them.
>
> Ah, thanks for that catch. And I see that's exactly what
> `html-quick-keys' is for. I've regularized the keybindings accordingly.

And! There was garbage in the re-search-backwards calls. Should look
like this (also removed it from the span tag, which shouldn't start a
newline):

(define-skeleton html-list-item
  "HTML list item tag."
  nil
  (if (re-search-backward "^[[:blank:]]+" (point-at-bol) t) nil '\n)
  "<li>" _ (if sgml-xml-mode "</li>"))

(define-skeleton html-paragraph
  "HTML paragraph tag."
  nil
  (if (re-search-backward "^[[:blank:]]+" (point-at-bol) t) nil ?\n)
  "<p>" > _ (if sgml-xml-mode "</p>"))

(define-skeleton html-div
  "HTML div tag."
  nil
  (if (re-search-backward "^[[:blank:]]+" (point-at-bol) t) nil ?\n)
  "<div>" > \n _ \n "</div>" >)

(define-skeleton html-span
  "HTML span tag."
  nil
  "<span>" > _ "</span>")


Reply | Threaded
Open this post in threaded view
|

Re: some proposed tweaks to HTML mode

Stefan Monnier
In reply to this post by Eric Abrahamsen-2
> I've always found vanilla HTML mode kind of annoying to use, and finally
> tried to figure out why. (I assume everyone is using some fancier HTML
> authoring mode, but that's no reason not to fix this one.)

I use nxml-mode, but I'd like to make it use more of sgml/html-mode
(e.g. for indentation), so this is very welcome in any case.

> @@ -1962,7 +1965,7 @@ html-tag-alist
>        ("dd" ,(not sgml-xml-mode))
>        ("del" nil ("cite") ("datetime"))
>        ("dfn")
> -      ("div")
> +      ("div" nil ("class") ("id"))
>
> The <div> and <span> elements have literally no use except for a place
> to hang the "class" and/or "id" attribute, so I don't see why we don't
> accept those here. In fact, I'd rather just automatically add "class"
> and "id" as potential attributes on all the tags.

Indeed, `class` and `id` should be automatically added to all elements
(i.e. not in the above list but directly in the sgml-attributes code).
But for many elements it makes sense to not query any attributes at all
since they almost never have any attributes (not even "class" nor "id").

> @@ -2451,26 +2455,38 @@ html-ordered-list
>    nil
>    "<ol>" \n
>    "<li>" _ (if sgml-xml-mode "</li>") \n
> -  "</ol>")
> +  "</ol>" >)

Since the skeleton uses several lines in all cases, I think it makes
sense to use \n rather than > at the very end, to make sure that any
text that follows is placed on another line.
Same for the <ul> element, of course.
[ I like to put consecutive close tags all on the same line, but I'd
still prefer the skeleton to use \n here, even if it occasionally forces
me to then remove the line break.  ]

>  (define-skeleton html-list-item
>    "HTML list item tag."
>    nil
> -  (if (bolp) nil '\n)
> +  (if (re-search-backward "^[[:blank:]]+") nil '\n)
>    "<li>" _ (if sgml-xml-mode "</li>"))

I see you already fixed it to use save-excursion, but it's worth
mentioning that there's `sgml-at-indentation-p` for that.

Also, I wonder what this whole (if (bolp) nil '\n) is about.  I suspect
that it's a remnant from many years when skeleton's \n was more naive:
originally, \n in skeletons always inserted a newline, but I changed at
many years ago such that a \n as first element behaves differently
(basically it behaves like (if (sgml-at-indentation-p) nil '\n).
Similarly, a \n at the very end of the skeleton only inserts a newline
if we're not already followed by a newline.

IOW, I think you can replace (if (sgml-at-indentation-p) nil '\n) with
just \n

> The above changes (in addition to adding skeletons for div and span
> tags) assume that what we want is a nicely indented HTML buffer. Nested
> divs should be indented, etc. This is what I want when writing HTML, but
> it's an assumption that might not be welcomed by everyone.

I for one welcome it.


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: some proposed tweaks to HTML mode

Eric Abrahamsen-2
Stefan Monnier <[hidden email]> writes:

>> I've always found vanilla HTML mode kind of annoying to use, and finally
>> tried to figure out why. (I assume everyone is using some fancier HTML
>> authoring mode, but that's no reason not to fix this one.)
>
> I use nxml-mode, but I'd like to make it use more of sgml/html-mode
> (e.g. for indentation), so this is very welcome in any case.
>
>> @@ -1962,7 +1965,7 @@ html-tag-alist
>>        ("dd" ,(not sgml-xml-mode))
>>        ("del" nil ("cite") ("datetime"))
>>        ("dfn")
>> -      ("div")
>> +      ("div" nil ("class") ("id"))
>>
>> The <div> and <span> elements have literally no use except for a place
>> to hang the "class" and/or "id" attribute, so I don't see why we don't
>> accept those here. In fact, I'd rather just automatically add "class"
>> and "id" as potential attributes on all the tags.
>
> Indeed, `class` and `id` should be automatically added to all elements
> (i.e. not in the above list but directly in the sgml-attributes code).
> But for many elements it makes sense to not query any attributes at all
> since they almost never have any attributes (not even "class" nor "id").

I was also thinking `sgml-attributes', but I'm less sure about excluding
elements. Changing the structure of `html-tag-alist' to carry this
information seems like it would be asking for backward-compatibility
pain -- maybe a new defcustom listing elements that shouldn't get
attributes?

>> @@ -2451,26 +2455,38 @@ html-ordered-list
>>    nil
>>    "<ol>" \n
>>    "<li>" _ (if sgml-xml-mode "</li>") \n
>> -  "</ol>")
>> +  "</ol>" >)
>
> Since the skeleton uses several lines in all cases, I think it makes
> sense to use \n rather than > at the very end, to make sure that any
> text that follows is placed on another line.
> Same for the <ul> element, of course.
> [ I like to put consecutive close tags all on the same line, but I'd
> still prefer the skeleton to use \n here, even if it occasionally forces
> me to then remove the line break.  ]
>
>>  (define-skeleton html-list-item
>>    "HTML list item tag."
>>    nil
>> -  (if (bolp) nil '\n)
>> +  (if (re-search-backward "^[[:blank:]]+") nil '\n)
>>    "<li>" _ (if sgml-xml-mode "</li>"))
>
> I see you already fixed it to use save-excursion, but it's worth
> mentioning that there's `sgml-at-indentation-p` for that.
>
> Also, I wonder what this whole (if (bolp) nil '\n) is about.  I suspect
> that it's a remnant from many years when skeleton's \n was more naive:
> originally, \n in skeletons always inserted a newline, but I changed at
> many years ago such that a \n as first element behaves differently
> (basically it behaves like (if (sgml-at-indentation-p) nil '\n).
> Similarly, a \n at the very end of the skeleton only inserts a newline
> if we're not already followed by a newline.
>
> IOW, I think you can replace (if (sgml-at-indentation-p) nil '\n) with
> just \n

Making sure I follow you, the definition of eg `html-ordered-list' would
then look like:

(define-skeleton html-ordered-list
  "HTML ordered list tags."
  nil
  \n "<ol>" \n
  "<li>" _ (if sgml-xml-mode "</li>") \n
  "</ol>" > \n)

Is that right? This does more or less the right thing, though it does in
some cases still insert a newline. In the text below, with point at the
vertical bar, the above definition still inserts another closing
newline:

<div>
  <div>|
   
  </div>
</div>

I found this bit of the Autotype manual confusing:

‘?\n’
     Insert a newline and align under current line, but not if this is
     the last element of a skeleton and the newline would be inserted at
     end of line, or this is the first element and the newline would be
     inserted at beginning of line.  Use newline character ‘?\n’ to
     prevent alignment.  Use ‘"\n"’ as the first or last string element
     of a skeleton to insert a newline unconditionally.

Here, "align" has nothing to do with "indent", right? This appears to
describe the behavior of ?\n as newline-and-align, but then goes on to
say we should use ?\n to *prevent* alignment.

>> The above changes (in addition to adding skeletons for div and span
>> tags) assume that what we want is a nicely indented HTML buffer. Nested
>> divs should be indented, etc. This is what I want when writing HTML, but
>> it's an assumption that might not be welcomed by everyone.
>
> I for one welcome it.

Great!


Reply | Threaded
Open this post in threaded view
|

Re: some proposed tweaks to HTML mode

Stefan Monnier
>> Indeed, `class` and `id` should be automatically added to all elements
>> (i.e. not in the above list but directly in the sgml-attributes code).
>> But for many elements it makes sense to not query any attributes at all
>> since they almost never have any attributes (not even "class" nor "id").
>
> I was also thinking `sgml-attributes', but I'm less sure about excluding
> elements.

I didn't mean to exclude elements.  I meant that if attributes are
queried then we do want to include `class` and `id`, but if they're not
then we shouldn't change it just on the premise that `class` or `id`
could be added.

> Making sure I follow you, the definition of eg `html-ordered-list' would
> then look like:
>
> (define-skeleton html-ordered-list
>   "HTML ordered list tags."
>   nil
>   \n "<ol>" \n
>   "<li>" _ (if sgml-xml-mode "</li>") \n
>   "</ol>" > \n)

Right.

> Is that right? This does more or less the right thing, though it does in
> some cases still insert a newline. In the text below, with point at the
> vertical bar, the above definition still inserts another closing
> newline:
>
> <div>
>   <div>|
>    
>   </div>
> </div>

I don't think it adds a closing newline any more than the current
code does: you start with point in front of 2 newlines and you end with
2 newlines after </ol>.

> I found this bit of the Autotype manual confusing:
>
> ‘?\n’
>      Insert a newline and align under current line, but not if this is
>      the last element of a skeleton and the newline would be inserted at
>      end of line, or this is the first element and the newline would be
>      inserted at beginning of line.  Use newline character ‘?\n’ to
>      prevent alignment.  Use ‘"\n"’ as the first or last string element
>      of a skeleton to insert a newline unconditionally.
>
> Here, "align" has nothing to do with "indent", right?

Right.

> This appears to
> describe the behavior of ?\n as newline-and-align, but then goes on to
> say we should use ?\n to *prevent* alignment.

Indeed, \n is the magic one while ?\n just inserts the LF char
without any extra processing (just like "\n").


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: some proposed tweaks to HTML mode

Eric Abrahamsen-2
Stefan Monnier <[hidden email]> writes:

>>> Indeed, `class` and `id` should be automatically added to all elements
>>> (i.e. not in the above list but directly in the sgml-attributes code).
>>> But for many elements it makes sense to not query any attributes at all
>>> since they almost never have any attributes (not even "class" nor "id").
>>
>> I was also thinking `sgml-attributes', but I'm less sure about excluding
>> elements.
>
> I didn't mean to exclude elements.  I meant that if attributes are
> queried then we do want to include `class` and `id`, but if they're not
> then we shouldn't change it just on the premise that `class` or `id`
> could be added.

Yes, that makes more sense.

>> Making sure I follow you, the definition of eg `html-ordered-list' would
>> then look like:
>>
>> (define-skeleton html-ordered-list
>>   "HTML ordered list tags."
>>   nil
>>   \n "<ol>" \n
>>   "<li>" _ (if sgml-xml-mode "</li>") \n
>>   "</ol>" > \n)
>
> Right.
>
>> Is that right? This does more or less the right thing, though it does in
>> some cases still insert a newline. In the text below, with point at the
>> vertical bar, the above definition still inserts another closing
>> newline:
>>
>> <div>
>>   <div>|
>>    
>>   </div>
>> </div>
>
> I don't think it adds a closing newline any more than the current
> code does: you start with point in front of 2 newlines and you end with
> 2 newlines after </ol>.

That was some sort of thinko, or temporary blindness.

>> I found this bit of the Autotype manual confusing:
>>
>> ‘?\n’
>>      Insert a newline and align under current line, but not if this is
>>      the last element of a skeleton and the newline would be inserted at
>>      end of line, or this is the first element and the newline would be
>>      inserted at beginning of line.  Use newline character ‘?\n’ to
>>      prevent alignment.  Use ‘"\n"’ as the first or last string element
>>      of a skeleton to insert a newline unconditionally.
>>
>> Here, "align" has nothing to do with "indent", right?
>
> Right.
>
>> This appears to
>> describe the behavior of ?\n as newline-and-align, but then goes on to
>> say we should use ?\n to *prevent* alignment.
>
> Indeed, \n is the magic one while ?\n just inserts the LF char
> without any extra processing (just like "\n").

Okay, I will work up the html-mode changes as a proper bug report in the
next few days, and maybe do another doc patch for this bit of the
manual.

Thanks,
Eric


Reply | Threaded
Open this post in threaded view
|

Re: some proposed tweaks to HTML mode

Eric Abrahamsen-2
Eric Abrahamsen <[hidden email]> writes:


[...]

>> I don't think it adds a closing newline any more than the current
>> code does: you start with point in front of 2 newlines and you end with
>> 2 newlines after </ol>.
>
> That was some sort of thinko, or temporary blindness.

(Emacs has actually-blind users, this was kind of a stupid thing to say,
sorry.)


Reply | Threaded
Open this post in threaded view
|

Re: some proposed tweaks to HTML mode

Eric Abrahamsen-2
In reply to this post by Eric Abrahamsen-2
Eric Abrahamsen <[hidden email]> writes:

> Stefan Monnier <[hidden email]> writes:
>
>>>> Indeed, `class` and `id` should be automatically added to all elements
>>>> (i.e. not in the above list but directly in the sgml-attributes code).
>>>> But for many elements it makes sense to not query any attributes at all
>>>> since they almost never have any attributes (not even "class" nor "id").
>>>
>>> I was also thinking `sgml-attributes', but I'm less sure about excluding
>>> elements.
>>
>> I didn't mean to exclude elements.  I meant that if attributes are
>> queried then we do want to include `class` and `id`, but if they're not
>> then we shouldn't change it just on the premise that `class` or `id`
>> could be added.
>
> Yes, that makes more sense.

Patches are in at bug #35517.