bug#43242: [PATCH] Fix CSS completion bug

bug#43242: [PATCH] Fix CSS completion bug

[Philip K.]

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


Hi,

I like to write CSS rules on one line, but Emacs completion-at-point
system always seems to fail if there is more than one rule per line. It
seems the reason was that css--complete-property-value as part of
css-completion-at-point falsely reported a match, because it ignored the
semi-colon. This means that in situations like these

            color: red; padd
                            ^
                        point here

the CAPF backend assumed a "color" value should be completed, even
though the user probably wants the "padd" string to be completed.

The patch below fixes this by making sure css--complete-property-value
doesn't ignore the semi-colon.

-- 
	Philip K.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-CSS-completion-with-multiple-rules-on-one-line.patch --]
[-- Type: text/x-patch, Size: 1439 bytes --]

From d452cd225845000f278e39c02796cf323f931925 Mon Sep 17 00:00:00 2001
From: Philip K <philipk@posteo.net>
Date: Sun, 6 Sep 2020 14:42:03 +0200
Subject: [PATCH] Allow CSS completion with multiple rules on one line

* css-mode.el (css--complete-property-value): Check for semi-colon
  when completing property values
---
 lisp/textmodes/css-mode.el | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index cc5879880c..d0882c68d0 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -1357,13 +1357,11 @@ css--property-values
 (defun css--complete-property-value ()
   "Complete property value at point."
   (let ((property
-         (save-excursion
-           (re-search-backward ":[^/]" (line-beginning-position) t)
-           (when (eq (char-after) ?:)
-             (let ((property-end (point)))
-               (skip-chars-backward "-[:alnum:]")
-               (let ((prop (buffer-substring (point) property-end)))
-                 (car (member prop css-property-ids))))))))
+         (save-match-data
+           (and (looking-back "\\([[:alnum:]-]+\\):[^/][^;]*"
+                              (line-beginning-position) t)
+                (car (member (match-string-no-properties 1)
+                             css-property-ids))))))
     (when property
       (let ((end (point)))
         (save-excursion
-- 
2.26.2

bug#43242: [PATCH] Fix CSS completion bug

[Lars Ingebrigtsen]

"Philip K." <philipk@posteo.net> writes:

> The patch below fixes this by making sure css--complete-property-value
> doesn't ignore the semi-colon.

[...]

> +         (save-match-data
> +           (and (looking-back "\\([[:alnum:]-]+\\):[^/][^;]*"

Looks good...  but is there any particular reason to save the match data
here?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#43242: [PATCH] Fix CSS completion bug

[Philip K.]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> "Philip K." <philipk@posteo.net> writes:
>
>> The patch below fixes this by making sure css--complete-property-value
>> doesn't ignore the semi-colon.
>
> [...]
>
>> +         (save-match-data
>> +           (and (looking-back "\\([[:alnum:]-]+\\):[^/][^;]*"
>
> Looks good...  but is there any particular reason to save the match data
> here?

Well, it's being used in the next line in the member call to extract the
record from css-property-ids.

-- 
	Philip K.

bug#43242: [PATCH] Fix CSS completion bug

[Lars Ingebrigtsen]

"Philip K." <philipk@posteo.net> writes:

> Well, it's being used in the next line in the member call to extract the
> record from css-property-ids.

I think there may be a slight misunderstanding about what the point of
save-match-data is.  Here's the code:

+         (save-match-data
+           (and (looking-back "\\([[:alnum:]-]+\\):[^/][^;]*"
+                              (line-beginning-position) t)
+                (car (member (match-string-no-properties 1)
+                             css-property-ids))))))

The regexp operators set a global match state, but is you use
save-match-data, the previous global state is restored after the form
ends.  So save-match-data is used to preserve the global state -- it's
not necessary if you don't care about overwriting it, and overwriting it
is the norm.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#43242: [PATCH] Fix CSS completion bug

[Philip K.]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> "Philip K." <philipk@posteo.net> writes:
>
>> Well, it's being used in the next line in the member call to extract the
>> record from css-property-ids.
>
> I think there may be a slight misunderstanding about what the point of
> save-match-data is.  Here's the code:
>
> +         (save-match-data
> +           (and (looking-back "\\([[:alnum:]-]+\\):[^/][^;]*"
> +                              (line-beginning-position) t)
> +                (car (member (match-string-no-properties 1)
> +                             css-property-ids))))))
>
> The regexp operators set a global match state, but is you use
> save-match-data, the previous global state is restored after the form
> ends.  So save-match-data is used to preserve the global state -- it's
> not necessary if you don't care about overwriting it, and overwriting it
> is the norm.

Oh, I wasn't familiar with that convention. I always save the match data
to avoid bugs bugs down the line. If it's wished for, I can resubmit the
patch without the save-match-data.

-- 
	Philip K.

bug#43242: [PATCH] Fix CSS completion bug

[Lars Ingebrigtsen]

"Philip K." <philipk@posteo.net> writes:

> Oh, I wasn't familiar with that convention. I always save the match data
> to avoid bugs bugs down the line. If it's wished for, I can resubmit the
> patch without the save-match-data.

Yes, please.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#43242: [PATCH] Fix CSS completion bug

[Philip K.]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> "Philip K." <philipk@posteo.net> writes:
>
>> Oh, I wasn't familiar with that convention. I always save the match data
>> to avoid bugs bugs down the line. If it's wished for, I can resubmit the
>> patch without the save-match-data.
>
> Yes, please.

See below.

I also took the liberty to restructure the code a bit, hopefully
making it easier to read. Hope that's OK.

-- 
	Philip K.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-CSS-completion-with-multiple-rules-on-one-line.patch --]
[-- Type: text/x-patch, Size: 1775 bytes --]

From 87244a9bfb9a285cf7df547465014527b2260027 Mon Sep 17 00:00:00 2001
From: Philip K <philipk@posteo.net>
Date: Sun, 6 Sep 2020 14:42:03 +0200
Subject: [PATCH] Allow CSS completion with multiple rules on one line

* css-mode.el (css--complete-property-value): Check for semi-colon
  when completing property values
---
 lisp/textmodes/css-mode.el | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index cc5879880c..9cddc17eaa 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -1356,21 +1356,17 @@ css--property-values
 
 (defun css--complete-property-value ()
   "Complete property value at point."
-  (let ((property
-         (save-excursion
-           (re-search-backward ":[^/]" (line-beginning-position) t)
-           (when (eq (char-after) ?:)
-             (let ((property-end (point)))
-               (skip-chars-backward "-[:alnum:]")
-               (let ((prop (buffer-substring (point) property-end)))
-                 (car (member prop css-property-ids))))))))
+  (let ((property (and (looking-back "\\([[:alnum:]-]+\\):[^/][^;]*"
+                                     (line-beginning-position) t)
+                       (member (match-string-no-properties 1)
+                               css-property-ids))))
     (when property
       (let ((end (point)))
         (save-excursion
           (skip-chars-backward "[:graph:]")
           (list (point) end
                 (append '("inherit" "initial" "unset")
-                        (css--property-values property))))))))
+                        (css--property-values (car property)))))))))
 
 (defvar css--html-tags (mapcar #'car html-tag-alist)
   "List of HTML tags.
-- 
2.26.2

bug#43242: [PATCH] Fix CSS completion bug

[Lars Ingebrigtsen]

"Philip K." <philipk@posteo.net> writes:

> I also took the liberty to restructure the code a bit, hopefully
> making it easier to read. Hope that's OK.

Looks good to me (and I tested it slightly in a CSS file); applied to
Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no