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

["Simen Heggestøyl" <simenheg@gmail.com>]

[-- 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