bug#23356: [PATCH] Cache results of `css--property-values'

bug#23356: [PATCH] Cache results of `css--property-values'

[Simen Heggestøyl]

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

During one of the iterations for 269d5631 (Support completion of
attribute values in CSS mode), the code that actually updates the
`css--property-value-cache' cache in `css--property-values' got lost.
The attached patch reintroduces it along with a regression test.

--Simen

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Cache-results-of-css-property-values.patch --]
[-- Type: text/x-patch, Size: 2503 bytes --]

From 03296601a423ddd0a9a82bc6baf91b484c1ca92b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@gmail.com>
Date: Sun, 24 Apr 2016 11:03:22 +0200
Subject: [PATCH] Cache results of `css--property-values'

* lisp/textmodes/css-mode.el (css--property-values): Cache computed
values.

* test/lisp/textmodes/css-mode-tests.el (css-test-property-value-cache):
New regression test for the above.
---
 lisp/textmodes/css-mode.el            | 16 +++++++++-------
 test/lisp/textmodes/css-mode-tests.el |  7 +++++++
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index 2a61fe3..8a4daac 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -792,13 +792,15 @@ css--property-values
 Completion candidates are looked up in `css-property-alist' by
 the string PROPERTY."
   (or (gethash property css--property-value-cache)
-      (seq-mapcat
-       (lambda (value)
-         (if (stringp value)
-             (list value)
-           (or (css--value-class-lookup value)
-               (css--property-values (symbol-name value)))))
-       (cdr (assoc property css-property-alist)))))
+      (let ((values
+             (seq-mapcat
+              (lambda (value)
+                (if (stringp value)
+                    (list value)
+                  (or (css--value-class-lookup value)
+                      (css--property-values (symbol-name value)))))
+              (cdr (assoc property css-property-alist)))))
+        (puthash property values css--property-value-cache))))
 
 (defun css--complete-property-value ()
   "Complete property value at point."
diff --git a/test/lisp/textmodes/css-mode-tests.el b/test/lisp/textmodes/css-mode-tests.el
index 9c5953d..805364c 100644
--- a/test/lisp/textmodes/css-mode-tests.el
+++ b/test/lisp/textmodes/css-mode-tests.el
@@ -57,6 +57,13 @@
   ;; because it refers to the value class of the same name.
   (should (= (length (css--property-values "color")) 18)))
 
+(ert-deftest css-test-property-value-cache ()
+  "Test that `css--property-value-cache' is in use."
+  (should-not (gethash "word-wrap" css--property-value-cache))
+  (let ((word-wrap-values (css--property-values "word-wrap")))
+    (should (equal (gethash "word-wrap" css--property-value-cache)
+                   word-wrap-values))))
+
 (ert-deftest css-test-value-class-lookup ()
   (should
    (equal (sort (css--value-class-lookup 'position) #'string-lessp)
-- 
2.8.0.rc3

bug#23356: [PATCH] Cache results of `css--property-values'

[Dmitry Gutov]

On 04/24/2016 03:44 PM, Simen Heggestøyl wrote:
> During one of the iterations for 269d5631 (Support completion of
> attribute values in CSS mode), the code that actually updates the
> `css--property-value-cache' cache in `css--property-values' got lost.
> The attached patch reintroduces it along with a regression test.

LGTM.

You should be able to push this kind of changes without going through 
review anyway.

bug#23356: [PATCH] Cache results of `css--property-values'

[Simen Heggestøyl]

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

On Sun, Apr 24, 2016 at 3:00 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> LGTM.

Thanks for the review, installed in master.

> You should be able to push this kind of changes without going through 
> review anyway.

I thought so too, but I wasn't entirely sure. I'll adjust my threshold
then for what I can go ahead with without a review.

-- Simen

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