bug#24389: [PATCH] Support completion of classes and IDs in CSS mode

bug#24389: [PATCH] Support completion of classes and IDs in CSS mode

[Simen Heggestøyl]

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

Hello.

I've got a seemingly working version of completion of HTML class names
and IDs in CSS mode buffers.

The idea is that CSS mode asks other open buffers if they are able to
produce completion candidates for HTML classes and IDs by checking
whether the functions `html-class-extractor-function' or
`html-id-extractor-function' are bound. I've included implementations
for these in HTML mode using libxml. My hope is that other modes, such
as Web mode [1], will be be able to implement their own extractor
functions.

What do you think?

-- Simen


[1] http://web-mode.org/



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Support-completion-of-classes-and-IDs-in-CSS-mode.patch --]
[-- Type: text/x-patch, Size: 6458 bytes --]

From 45503ade03b277e2f66f2259b0436aa35a162bf2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@gmail.com>
Date: Sat, 25 Jun 2016 15:12:09 +0200
Subject: [PATCH] Support completion of classes and IDs in CSS mode

* lisp/textmodes/css-mode.el (css--complete-html-tag): New function
for completing HTML tags, which was previously the responsibility of
`css--complete-selector'.
(css--foreign-completion-cache): New variable holding a cache for
completions provided by other buffers.
(css--foreign-completions): New function for retrieving completions
from other buffers.
(css--complete-selector): Support completing HTML IDs and classes from
other buffers in addition to completing HTML tags.

* lisp/textmodes/sgml-mode.el (html-current-buffer-classes): New
function returning a list of class names used in the current buffer.
(html-current-buffer-ids): New function returning a list of IDs used
in the current buffer.
(html-class-extractor-function): New variable holding the function to
use for class name extraction.
(html-id-extractor-function): New variable holding the function to use
for ID extraction.
(html-mode): Set `html-class-extractor-function' and
`html-id-extractor-function' to `html-current-buffer-classes' and
`html-current-buffer-ids' respectively.
---
 lisp/textmodes/css-mode.el  | 56 +++++++++++++++++++++++++++++++++++++++------
 lisp/textmodes/sgml-mode.el | 29 +++++++++++++++++++++++
 2 files changed, 78 insertions(+), 7 deletions(-)

diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index 4d8170e..20f3ff2 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -30,7 +30,6 @@
 ;; - electric ; and }
 ;; - filling code with auto-fill-mode
 ;; - fix font-lock errors with multi-line selectors
-;; - support completion of user-defined classes names and IDs
 
 ;;; Code:
 
@@ -864,16 +863,59 @@ css--nested-selectors-allowed
   "Non-nil if nested selectors are allowed in the current mode.")
 (make-variable-buffer-local 'css--nested-selectors-allowed)
 
-;; TODO: Currently only supports completion of HTML tags.  By looking
-;; at open HTML mode buffers we should be able to provide completion
-;; of user-defined classes and IDs too.
+(defun css--complete-html-tag ()
+  "Complete HTML tag at point."
+  (save-excursion
+    (let ((end (point)))
+      (skip-chars-backward "-[:alnum:]")
+      (list (point) end css--html-tags))))
+
+(defvar css--foreign-completion-cache
+  (list 'html-class-extractor-function (make-hash-table :test 'equal)
+        'html-id-extractor-function (make-hash-table :test 'equal))
+  "Cache of completions provided by other buffers.
+This is a property list where each property is the name of an
+extractor function and the associated value is a hash table
+serving as a cache for that function.")
+
+(defun css--foreign-completions (extractor)
+  "Return a list of completions provided by other buffers.
+EXTRACTOR should be the name of a function that may be defined in
+one or more buffers.  In each of the buffers where EXTRACTOR is
+defined, EXTRACTOR is called and the results are accumulated into
+a list."
+  (seq-uniq
+   (seq-mapcat
+    (lambda (buf)
+      (with-current-buffer buf
+        (when (boundp extractor)
+          (let ((cache
+                 (plist-get css--foreign-completion-cache extractor)))
+            (if cache
+                (let ((hash (buffer-hash buf)))
+                  (or (gethash hash cache)
+                      (puthash hash (funcall (symbol-value extractor))
+                               cache)))
+              (funcall (symbol-value extractor)))))))
+    (buffer-list))))
+
 (defun css--complete-selector ()
   "Complete part of a CSS selector at point."
   (when (or (= (nth 0 (syntax-ppss)) 0) css--nested-selectors-allowed)
-    (save-excursion
-      (let ((end (point)))
+    (let ((end (point)))
+      (save-excursion
         (skip-chars-backward "-[:alnum:]")
-        (list (point) end css--html-tags)))))
+        (let ((start-char (char-before)))
+          (list
+           (point) end
+           (completion-table-dynamic
+            (lambda (_)
+              (cond
+               ((eq start-char ?.) (css--foreign-completions
+                                    'html-class-extractor-function))
+               ((eq start-char ?#) (css--foreign-completions
+                                    'html-id-extractor-function))
+               (t css--html-tags))))))))))
 
 (defun css-completion-at-point ()
   "Complete current symbol at point.
diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index 990c09b..5e943de 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -32,6 +32,9 @@
 
 ;;; Code:
 
+(require 'dom)
+(require 'seq)
+(require 'subr-x)
 (eval-when-compile
   (require 'skeleton)
   (require 'cl-lib))
@@ -2168,6 +2171,27 @@ html-current-defun-name
 	 nil t)
 	(match-string-no-properties 1))))
 
+(defun html-current-buffer-classes ()
+  "Return a list of class names used in the current buffer."
+  (let ((dom (libxml-parse-html-region (point-min) (point-max))))
+    (seq-mapcat
+     (lambda (el)
+       (when-let (class-list (cdr (assq 'class (dom-attributes el))))
+         (split-string class-list)))
+     (dom-by-class dom ""))))
+
+(defun html-current-buffer-ids ()
+  "Return a list of IDs used in the current buffer."
+  (let ((dom (libxml-parse-html-region (point-min) (point-max))))
+    (seq-mapcat
+     (lambda (el)
+       (when-let (id-list (cdr (assq 'id (dom-attributes el))))
+         (split-string id-list)))
+     (dom-by-id dom ""))))
+
+(defvar html-class-extractor-function)
+(defvar html-id-extractor-function)
+
 \f
 ;;;###autoload
 (define-derived-mode html-mode sgml-mode '(sgml-xml-mode "XHTML" "HTML")
@@ -2218,6 +2242,11 @@ html-mode
   (setq-local add-log-current-defun-function #'html-current-defun-name)
   (setq-local sentence-end-base "[.?!][]\"'”)}]*\\(<[^>]*>\\)*")
 
+  (when (fboundp 'libxml-parse-html-region)
+    (setq-local html-class-extractor-function
+                #'html-current-buffer-classes)
+    (setq-local html-id-extractor-function #'html-current-buffer-ids))
+
   (setq imenu-create-index-function 'html-imenu-index)
 
   (setq-local sgml-empty-tags
-- 
2.9.3

bug#24389: [PATCH] Support completion of classes and IDs in CSS mode

[Stefan Monnier]

> What do you think?

I like the general idea, but have some nitpicks below.

> +(defun css--complete-html-tag ()
> +  "Complete HTML tag at point."
> +  (save-excursion
> +    (let ((end (point)))
> +      (skip-chars-backward "-[:alnum:]")
> +      (list (point) end css--html-tags))))

I don't see where this is used.

> +(defvar css--foreign-completion-cache
> +  (list 'html-class-extractor-function (make-hash-table :test 'equal)
> +        'html-id-extractor-function (make-hash-table :test 'equal))

I urge you to use alists rather than plist whenever you can.  I only
recommend plists for those rare cases where they're likely to often be
written down by end-users who'd be bothered by the need to add dots
and parentheses.

> +(defun css--foreign-completions (extractor)
> +  "Return a list of completions provided by other buffers.
> +EXTRACTOR should be the name of a function that may be defined in
> +one or more buffers.  In each of the buffers where EXTRACTOR is
> +defined, EXTRACTOR is called and the results are accumulated into
> +a list."
> +  (seq-uniq
> +   (seq-mapcat
> +    (lambda (buf)
> +      (with-current-buffer buf
> +        (when (boundp extractor)
> +          (let ((cache
> +                 (plist-get css--foreign-completion-cache extractor)))
> +            (if cache
> +                (let ((hash (buffer-hash buf)))
> +                  (or (gethash hash cache)
> +                      (puthash hash (funcall (symbol-value extractor))
> +                               cache)))
> +              (funcall (symbol-value extractor)))))))
> +    (buffer-list))))

I think this design won't work well.  How 'bout instead just always
calling the function and let the other side handle the caching?

This way the cache can work correctly even in case the list of
completions depends on sources other than the buffer's content (a case
which even your costly buffer-hash won't catch).  Also, it means that
depending on the cost of the extraction itself, the cache may be flushed
more or less eagerly.

> +(defvar html-class-extractor-function)
> +(defvar html-id-extractor-function)

The definition of those global variables seems to be missing.
IIUC they should be in css-mode.el, so their name should start with
"css-" I think (tho I agree that it's debatable).

In any case I think it's important that their global value be a valid
function rather than nil (it should probably be #'ignore).

Also, if the caching is moved to those functions, then their name should
not say "extractor" but should simply indicate that those functions
should return the list of classes/ids.  E.g. css-class-list-function.


        Stefan

bug#24389: [PATCH] Support completion of classes and IDs in CSS mode

[Simen Heggestøyl]

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

Hi Stefan, thanks for the feedback.

On Wed, Sep 7, 2016 at 10:01 PM, Stefan Monnier 
<monnier@IRO.UMontreal.CA> wrote:
> I don't see where this is used.

Oops, that was left over from an earlier revision.

> I urge you to use alists rather than plist whenever you can.  I only
> recommend plists for those rare cases where they're likely to often be
> written down by end-users who'd be bothered by the need to add dots
> and parentheses.

Noted, thanks.

> I think this design won't work well.  How 'bout instead just always
> calling the function and let the other side handle the caching?
> 
> This way the cache can work correctly even in case the list of
> completions depends on sources other than the buffer's content (a case
> which even your costly buffer-hash won't catch).  Also, it means that
> depending on the cost of the extraction itself, the cache may be 
> flushed
> more or less eagerly.

True. Considering the things you've mentioned, I also think it will be
much better to let the other side (optionally) handle their own
cache. Please see the revised patch.

> The definition of those global variables seems to be missing.
> IIUC they should be in css-mode.el, so their name should start with
> "css-" I think (tho I agree that it's debatable).

Hm, is it wrong to let them stay void, and then check whether they're
bound in each buffer we visit?

I ran som benchmarks to assess whether the expensive buffer-hash cache
is worth it, results follow below. All the tests were run with 583 HTML
mode buffers open, with content totaling 7.5 MB.

* No cache:
(benchmark 1 '(css--foreign-completions 'css-class-list-function))
    "Elapsed time: 1.597629s (0.797973s in 55 GCs)"
(benchmark 10 '(css--foreign-completions 'css-class-list-function))
    "Elapsed time: 15.322130s (7.589323s in 549 GCs)"

* Cold cache:
(benchmark 1 '(css--foreign-completions 'css-class-list-function))
    "Elapsed time: 1.609650s (0.786968s in 54 GCs)"

* Warm cache:
(benchmark 1 '(css--foreign-completions 'css-class-list-function))
    "Elapsed time: 0.471924s (0.102814s in 6 GCs)"
(benchmark 10 '(css--foreign-completions 'css-class-list-function))
    "Elapsed time: 4.376787s (0.981541s in 59 GCs)"

-- Simen

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Support-completion-of-classes-and-IDs-in-CSS-mode.patch --]
[-- Type: text/x-patch, Size: 7843 bytes --]

From bcb7d93d69bb8b536baffbe7558a7e89181c9c5b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@gmail.com>
Date: Sat, 25 Jun 2016 15:12:09 +0200
Subject: [PATCH] Support completion of classes and IDs in CSS mode

* lisp/textmodes/css-mode.el (css-class-list-function): New variable
holding the function to call for retrieving completions of class
names.
(css-id-list-function): New variable holding the function to call for
retrieving completions of IDs.
(css--foreign-completions): New function for retrieving completions
from other buffers.
(css--complete-selector): Support completing HTML class names and IDs
from other buffers in addition to completing HTML tags.

* lisp/textmodes/sgml-mode.el (html--buffer-classes-cache): New
variable holding a cache for `html-current-buffer-classes'.
(html--buffer-ids-cache): New variable holding a cache for
`html-current-buffer-ids'.
(html-current-buffer-classes): New function returning a list of class
names used in the current buffer.
(html-current-buffer-ids): New function returning a list of IDs used
in the current buffer.
(html-mode): Set `css-class-list-function' and `css-id-list-function'
to `html-current-buffer-classes' and `html-current-buffer-ids'
respectively.
---
 etc/NEWS                    |  6 +++--
 lisp/textmodes/css-mode.el  | 44 ++++++++++++++++++++++++++++------
 lisp/textmodes/sgml-mode.el | 58 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 99 insertions(+), 9 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 82eb2b8..2c1ce3b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -386,8 +386,10 @@ enables reading of shell initialization files.
 ** CSS mode
 
 ---
-*** Support for completing attribute values, at-rules, bang-rules, and
-HTML tags using the 'completion-at-point' command.
+*** Support for completing attribute values, at-rules, bang-rules,
+HTML tags, classes and IDs using the 'completion-at-point' command.
+Completion candidates for HTML classes and IDs are retrieved from open
+HTML mode buffers.
 
 +++
 ** Emacs now supports character name escape sequences in character and
diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index 4d8170e..4149a95 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -30,7 +30,6 @@
 ;; - electric ; and }
 ;; - filling code with auto-fill-mode
 ;; - fix font-lock errors with multi-line selectors
-;; - support completion of user-defined classes names and IDs
 
 ;;; Code:
 
@@ -864,16 +863,47 @@ css--nested-selectors-allowed
   "Non-nil if nested selectors are allowed in the current mode.")
 (make-variable-buffer-local 'css--nested-selectors-allowed)
 
-;; TODO: Currently only supports completion of HTML tags.  By looking
-;; at open HTML mode buffers we should be able to provide completion
-;; of user-defined classes and IDs too.
+(defvar css-class-list-function #'ignore
+  "Called to provide completions of class names.
+This can be bound by buffers that are able to suggest class name
+completions, such as HTML mode buffers.")
+
+(defvar css-id-list-function #'ignore
+  "Called to provide completions of IDs.
+This can be bound by buffers that are able to suggest ID
+completions, such as HTML mode buffers.")
+
+(defun css--foreign-completions (extractor)
+  "Return a list of completions provided by other buffers.
+EXTRACTOR should be the name of a function that may be defined in
+one or more buffers.  In each of the buffers where EXTRACTOR is
+defined, EXTRACTOR is called and the results are accumulated into
+a list of completions."
+  (seq-uniq
+   (seq-mapcat
+    (lambda (buf)
+      (with-current-buffer buf
+        (when (boundp extractor)
+          (funcall (symbol-value extractor)))))
+    (buffer-list))))
+
 (defun css--complete-selector ()
   "Complete part of a CSS selector at point."
   (when (or (= (nth 0 (syntax-ppss)) 0) css--nested-selectors-allowed)
-    (save-excursion
-      (let ((end (point)))
+    (let ((end (point)))
+      (save-excursion
         (skip-chars-backward "-[:alnum:]")
-        (list (point) end css--html-tags)))))
+        (let ((start-char (char-before)))
+          (list
+           (point) end
+           (completion-table-dynamic
+            (lambda (_)
+              (cond
+               ((eq start-char ?.)
+                (css--foreign-completions 'css-class-list-function))
+               ((eq start-char ?#)
+                (css--foreign-completions 'css-id-list-function))
+               (t css--html-tags))))))))))
 
 (defun css-completion-at-point ()
   "Complete current symbol at point.
diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index 990c09b..68ee7d0 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -32,6 +32,9 @@
 
 ;;; Code:
 
+(require 'dom)
+(require 'seq)
+(require 'subr-x)
 (eval-when-compile
   (require 'skeleton)
   (require 'cl-lib))
@@ -2168,6 +2171,55 @@ html-current-defun-name
 	 nil t)
 	(match-string-no-properties 1))))
 
+(defvar html--buffer-classes-cache nil
+  "Cache for `html-current-buffer-classes'.
+When set, this should be a cons cell where the CAR is the buffer
+hash as produced by `buffer-hash', and the CDR is the list of
+class names found in the buffer.")
+(make-variable-buffer-local 'html--buffer-classes-cache)
+
+(defvar html--buffer-ids-cache nil
+  "Cache for `html-current-buffer-ids'.
+When set, this should be a cons cell where the CAR is the buffer
+hash as produced by `buffer-hash', and the CDR is the list of IDs
+found in the buffer.")
+(make-variable-buffer-local 'html--buffer-ids-cache)
+
+(defun html-current-buffer-classes ()
+  "Return a list of class names used in the current buffer.
+The result is cached in `html--buffer-classes-cache'."
+  (let ((hash (buffer-hash)))
+    (if (equal (car html--buffer-classes-cache) hash)
+        (cdr html--buffer-classes-cache)
+      (let* ((dom (libxml-parse-html-region (point-min) (point-max)))
+             (classes
+              (seq-mapcat
+               (lambda (el)
+                 (when-let (class-list
+                            (cdr (assq 'class (dom-attributes el))))
+                   (split-string class-list)))
+               (dom-by-class dom ""))))
+        (setq-local html--buffer-classes-cache (cons hash classes))
+        classes))))
+
+(defun html-current-buffer-ids ()
+  "Return a list of IDs used in the current buffer.
+The result is cached in `html--buffer-ids-cache'."
+  (let ((hash (buffer-hash)))
+    (if (equal (car html--buffer-ids-cache) hash)
+        (cdr html--buffer-ids-cache)
+      (let* ((dom
+              (libxml-parse-html-region (point-min) (point-max)))
+             (ids
+              (seq-mapcat
+               (lambda (el)
+                 (when-let (id-list
+                            (cdr (assq 'id (dom-attributes el))))
+                   (split-string id-list)))
+               (dom-by-id dom ""))))
+        (setq-local html--buffer-ids-cache (cons hash ids))
+        ids))))
+
 \f
 ;;;###autoload
 (define-derived-mode html-mode sgml-mode '(sgml-xml-mode "XHTML" "HTML")
@@ -2218,6 +2270,12 @@ html-mode
   (setq-local add-log-current-defun-function #'html-current-defun-name)
   (setq-local sentence-end-base "[.?!][]\"'”)}]*\\(<[^>]*>\\)*")
 
+  (when (fboundp 'libxml-parse-html-region)
+    (defvar css-class-list-function)
+    (setq-local css-class-list-function #'html-current-buffer-classes)
+    (defvar css-id-list-function)
+    (setq-local css-id-list-function #'html-current-buffer-ids))
+
   (setq imenu-create-index-function 'html-imenu-index)
 
   (setq-local sgml-empty-tags
-- 
2.9.3

bug#24389: [PATCH] Support completion of classes and IDs in CSS mode

[Stefan Monnier]

>> The definition of those global variables seems to be missing.
>> IIUC they should be in css-mode.el, so their name should start with
>> "css-" I think (tho I agree that it's debatable).
> Hm, is it wrong to let them stay void, and then check whether they're
> bound in each buffer we visit?

What'd be the benefit?  You do want to `defvar' them in order to give
them a doctring (which means that if you want them void you have to do
extra gymnastic), and giving them a good default value means you can
call them without having to check the value beforehand (and that also
means you can use `add-function' on it).

> I ran som benchmarks to assess whether the expensive buffer-hash cache
> is worth it, results follow below.

`buffer-hash` is only a problem in large buffers.  But you can use
buffer-text-modified-tick to get a much quicker test (only important in
large buffers).  And you might not even need to test
buffer-text-modified-tick because you can often just flush the (relevant
part of) the cache(s) from an after-change-functions or from
syntax-propertize or ...

> +      (with-current-buffer buf
> +        (when (boundp extractor)
> +          (funcall (symbol-value extractor)))))

You don't need the boundp test here, AFAICT.


        Stefan

bug#24389: [PATCH] Support completion of classes and IDs in CSS mode

[Simen Heggestøyl]

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

Thanks again for your feedback.

On Sat, Sep 10, 2016 at 9:55 PM, Stefan Monnier 
<monnier@iro.umontreal.ca> wrote:
> What'd be the benefit?  You do want to `defvar' them in order to give
> them a doctring (which means that if you want them void you have to do
> extra gymnastic), and giving them a good default value means you can
> call them without having to check the value beforehand (and that also
> means you can use `add-function' on it).

Nothing, but I couldn't think of any downsides either before you
mentioned them. I see now that a `defvar' is better.

> `buffer-hash` is only a problem in large buffers.  But you can use
> buffer-text-modified-tick to get a much quicker test (only important 
> in
> large buffers).  And you might not even need to test
> buffer-text-modified-tick because you can often just flush the 
> (relevant
> part of) the cache(s) from an after-change-functions or from
> syntax-propertize or ...

OK, I changed it to use the buffer's tick counter instead. I also
changed `seq-uniq' to `delete-dups', which resulted in a massive
speedup. Some benchmarks using the same test files as before follow.

* With `seq-uniq':
(benchmark 10 '(css--foreign-completions 'css-class-list-function))
    "Elapsed time: 4.198944s (0.911449s in 60 GCs)"

* With `delete-dups':
(benchmark 10 '(css--foreign-completions 'css-class-list-function))
    "Elapsed time: 0.282890s (0.188205s in 10 GCs)"

As a side note, maybe a hashing strategy like the one `delete-dups'
uses would be good for `seq-uniq' too?

> You don't need the boundp test here, AFAICT.

Right, removed.

-- Simen

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Support-completion-of-classes-and-IDs-in-CSS-mode.patch --]
[-- Type: text/x-patch, Size: 7869 bytes --]

From e4aff3d72925be558f9278353d09b691edf39845 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@gmail.com>
Date: Sat, 25 Jun 2016 15:12:09 +0200
Subject: [PATCH] Support completion of classes and IDs in CSS mode

* lisp/textmodes/css-mode.el (css-class-list-function): New variable
holding the function to call for retrieving completions of class
names.
(css-id-list-function): New variable holding the function to call for
retrieving completions of IDs.
(css--foreign-completions): New function for retrieving completions
from other buffers.
(css--complete-selector): Support completing HTML class names and IDs
from other buffers in addition to completing HTML tags.

* lisp/textmodes/sgml-mode.el (html--buffer-classes-cache): New
variable holding a cache for `html-current-buffer-classes'.
(html--buffer-ids-cache): New variable holding a cache for
`html-current-buffer-ids'.
(html-current-buffer-classes): New function returning a list of class
names used in the current buffer.
(html-current-buffer-ids): New function returning a list of IDs used
in the current buffer.
(html-mode): Set `css-class-list-function' and `css-id-list-function'
to `html-current-buffer-classes' and `html-current-buffer-ids'
respectively.
---
 etc/NEWS                    |  6 +++--
 lisp/textmodes/css-mode.el  | 43 +++++++++++++++++++++++++++------
 lisp/textmodes/sgml-mode.el | 58 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 98 insertions(+), 9 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 9b992d0..ee49136 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -393,8 +393,10 @@ enables reading of shell initialization files.
 ** CSS mode
 
 ---
-*** Support for completing attribute values, at-rules, bang-rules, and
-HTML tags using the 'completion-at-point' command.
+*** Support for completing attribute values, at-rules, bang-rules,
+HTML tags, classes and IDs using the 'completion-at-point' command.
+Completion candidates for HTML classes and IDs are retrieved from open
+HTML mode buffers.
 
 +++
 ** Emacs now supports character name escape sequences in character and
diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index 4d8170e..53b3fa5 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -30,7 +30,6 @@
 ;; - electric ; and }
 ;; - filling code with auto-fill-mode
 ;; - fix font-lock errors with multi-line selectors
-;; - support completion of user-defined classes names and IDs
 
 ;;; Code:
 
@@ -864,16 +863,46 @@ css--nested-selectors-allowed
   "Non-nil if nested selectors are allowed in the current mode.")
 (make-variable-buffer-local 'css--nested-selectors-allowed)
 
-;; TODO: Currently only supports completion of HTML tags.  By looking
-;; at open HTML mode buffers we should be able to provide completion
-;; of user-defined classes and IDs too.
+(defvar css-class-list-function #'ignore
+  "Called to provide completions of class names.
+This can be bound by buffers that are able to suggest class name
+completions, such as HTML mode buffers.")
+
+(defvar css-id-list-function #'ignore
+  "Called to provide completions of IDs.
+This can be bound by buffers that are able to suggest ID
+completions, such as HTML mode buffers.")
+
+(defun css--foreign-completions (extractor)
+  "Return a list of completions provided by other buffers.
+EXTRACTOR should be the name of a function that may be defined in
+one or more buffers.  In each of the buffers where EXTRACTOR is
+defined, EXTRACTOR is called and the results are accumulated into
+a list of completions."
+  (delete-dups
+   (seq-mapcat
+    (lambda (buf)
+      (with-current-buffer buf
+        (funcall (symbol-value extractor))))
+    (buffer-list))))
+
 (defun css--complete-selector ()
   "Complete part of a CSS selector at point."
   (when (or (= (nth 0 (syntax-ppss)) 0) css--nested-selectors-allowed)
-    (save-excursion
-      (let ((end (point)))
+    (let ((end (point)))
+      (save-excursion
         (skip-chars-backward "-[:alnum:]")
-        (list (point) end css--html-tags)))))
+        (let ((start-char (char-before)))
+          (list
+           (point) end
+           (completion-table-dynamic
+            (lambda (_)
+              (cond
+               ((eq start-char ?.)
+                (css--foreign-completions 'css-class-list-function))
+               ((eq start-char ?#)
+                (css--foreign-completions 'css-id-list-function))
+               (t css--html-tags))))))))))
 
 (defun css-completion-at-point ()
   "Complete current symbol at point.
diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index 990c09b..43effef 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -32,6 +32,9 @@
 
 ;;; Code:
 
+(require 'dom)
+(require 'seq)
+(require 'subr-x)
 (eval-when-compile
   (require 'skeleton)
   (require 'cl-lib))
@@ -2168,6 +2171,55 @@ html-current-defun-name
 	 nil t)
 	(match-string-no-properties 1))))
 
+(defvar html--buffer-classes-cache nil
+  "Cache for `html-current-buffer-classes'.
+When set, this should be a cons cell where the CAR is the
+buffer's tick counter (as produced by `buffer-modified-tick'),
+and the CDR is the list of class names found in the buffer.")
+(make-variable-buffer-local 'html--buffer-classes-cache)
+
+(defvar html--buffer-ids-cache nil
+  "Cache for `html-current-buffer-ids'.
+When set, this should be a cons cell where the CAR is the
+buffer's tick counter (as produced by `buffer-modified-tick'),
+and the CDR is the list of class names found in the buffer.")
+(make-variable-buffer-local 'html--buffer-ids-cache)
+
+(defun html-current-buffer-classes ()
+  "Return a list of class names used in the current buffer.
+The result is cached in `html--buffer-classes-cache'."
+  (let ((tick (buffer-modified-tick)))
+    (if (eq (car html--buffer-classes-cache) tick)
+        (cdr html--buffer-classes-cache)
+      (let* ((dom (libxml-parse-html-region (point-min) (point-max)))
+             (classes
+              (seq-mapcat
+               (lambda (el)
+                 (when-let (class-list
+                            (cdr (assq 'class (dom-attributes el))))
+                   (split-string class-list)))
+               (dom-by-class dom ""))))
+        (setq-local html--buffer-classes-cache (cons tick classes))
+        classes))))
+
+(defun html-current-buffer-ids ()
+  "Return a list of IDs used in the current buffer.
+The result is cached in `html--buffer-ids-cache'."
+  (let ((tick (buffer-modified-tick)))
+    (if (eq (car html--buffer-ids-cache) tick)
+        (cdr html--buffer-ids-cache)
+      (let* ((dom
+              (libxml-parse-html-region (point-min) (point-max)))
+             (ids
+              (seq-mapcat
+               (lambda (el)
+                 (when-let (id-list
+                            (cdr (assq 'id (dom-attributes el))))
+                   (split-string id-list)))
+               (dom-by-id dom ""))))
+        (setq-local html--buffer-ids-cache (cons tick ids))
+        ids))))
+
 \f
 ;;;###autoload
 (define-derived-mode html-mode sgml-mode '(sgml-xml-mode "XHTML" "HTML")
@@ -2218,6 +2270,12 @@ html-mode
   (setq-local add-log-current-defun-function #'html-current-defun-name)
   (setq-local sentence-end-base "[.?!][]\"'”)}]*\\(<[^>]*>\\)*")
 
+  (when (fboundp 'libxml-parse-html-region)
+    (defvar css-class-list-function)
+    (setq-local css-class-list-function #'html-current-buffer-classes)
+    (defvar css-id-list-function)
+    (setq-local css-id-list-function #'html-current-buffer-ids))
+
   (setq imenu-create-index-function 'html-imenu-index)
 
   (setq-local sgml-empty-tags
-- 
2.9.3

bug#24389: [PATCH] Support completion of classes and IDs in CSS mode

[Stefan Monnier]

> OK, I changed it to use the buffer's tick counter instead. I also
> changed `seq-uniq' to `delete-dups', which resulted in a massive
> speedup. Some benchmarks using the same test files as before follow.
[...]
> As a side note, maybe a hashing strategy like the one `delete-dups'
> uses would be good for `seq-uniq' too?

I suggest you file a separate bug report for that issue.  Maybe using
a hash table like in delete-dups will do the trick, but I suspect that
the difference between using `delete` (coded in C) vs `seq-contains`
(which additionally performs an Elisp-funcall for each element) is also
a significant factor.

The patch looks good now, thanks,


        Stefan

bug#24389: [PATCH] Support completion of classes and IDs in CSS mode

[Simen Heggestøyl]

On Sat, Sep 17, 2016 at 2:35 PM, Stefan Monnier 
<monnier@IRO.UMontreal.CA> wrote:
> I suggest you file a separate bug report for that issue.

OK, I will.

> The patch looks good now, thanks,

Good! I've installed it in master, thanks for reviewing.

-- Simen

bug#24389: [PATCH] Support completion of classes and IDs in CSS mode

[Nicolas Petton]

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

Simen Heggestøyl <simenheg@gmail.com> writes:


> OK, I changed it to use the buffer's tick counter instead. I also
> changed `seq-uniq' to `delete-dups', which resulted in a massive
> speedup. Some benchmarks using the same test files as before follow.
>
> * With `seq-uniq':
> (benchmark 10 '(css--foreign-completions 'css-class-list-function))
>     "Elapsed time: 4.198944s (0.911449s in 60 GCs)"
>
> * With `delete-dups':
> (benchmark 10 '(css--foreign-completions 'css-class-list-function))
>     "Elapsed time: 0.282890s (0.188205s in 10 GCs)"
>
> As a side note, maybe a hashing strategy like the one `delete-dups'
> uses would be good for `seq-uniq' too?

Yes, seq-uniq could be optimized.  I'll look into it.

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]