GNU bug report logs - #43242
[PATCH] Fix CSS completion bug

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 43242 in the body.
You can then email your comments to 43242 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Philip K." <philipk <at> posteo.net>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Fix CSS completion bug
Date: Sun, 06 Sep 2020 14:51:03 +0200

[Message part 1 (text/plain, inline)]

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.

[0001-Allow-CSS-completion-with-multiple-rules-on-one-line.patch (text/x-patch, inline)]

From d452cd225845000f278e39c02796cf323f931925 Mon Sep 17 00:00:00 2001
From: Philip K <philipk <at> 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

Message #8 received at 43242 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "Philip K." <philipk <at> posteo.net>
Cc: 43242 <at> debbugs.gnu.org
Subject: Re: bug#43242: [PATCH] Fix CSS completion bug
Date: Sun, 06 Sep 2020 21:30:45 +0200

"Philip K." <philipk <at> 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

Message #11 received at 43242 <at> debbugs.gnu.org (full text, mbox):

From: "Philip K." <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 43242 <at> debbugs.gnu.org
Subject: Re: bug#43242: [PATCH] Fix CSS completion bug
Date: Sun, 06 Sep 2020 23:40:42 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> "Philip K." <philipk <at> 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.

Message #14 received at 43242 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "Philip K." <philipk <at> posteo.net>
Cc: 43242 <at> debbugs.gnu.org
Subject: Re: bug#43242: [PATCH] Fix CSS completion bug
Date: Sun, 06 Sep 2020 23:46:54 +0200

"Philip K." <philipk <at> 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

Message #17 received at 43242 <at> debbugs.gnu.org (full text, mbox):

From: "Philip K." <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 43242 <at> debbugs.gnu.org
Subject: Re: bug#43242: [PATCH] Fix CSS completion bug
Date: Sun, 06 Sep 2020 23:55:43 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> "Philip K." <philipk <at> 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.

Message #20 received at 43242 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "Philip K." <philipk <at> posteo.net>
Cc: 43242 <at> debbugs.gnu.org
Subject: Re: bug#43242: [PATCH] Fix CSS completion bug
Date: Sun, 06 Sep 2020 23:57:31 +0200

"Philip K." <philipk <at> 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

Message #23 received at 43242 <at> debbugs.gnu.org (full text, mbox):

From: "Philip K." <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 43242 <at> debbugs.gnu.org
Subject: Re: bug#43242: [PATCH] Fix CSS completion bug
Date: Mon, 07 Sep 2020 00:15:06 +0200

[Message part 1 (text/plain, inline)]

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> "Philip K." <philipk <at> 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.

[0001-Allow-CSS-completion-with-multiple-rules-on-one-line.patch (text/x-patch, inline)]

From 87244a9bfb9a285cf7df547465014527b2260027 Mon Sep 17 00:00:00 2001
From: Philip K <philipk <at> 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

Message #26 received at 43242 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "Philip K." <philipk <at> posteo.net>
Cc: 43242 <at> debbugs.gnu.org
Subject: Re: bug#43242: [PATCH] Fix CSS completion bug
Date: Mon, 07 Sep 2020 00:17:59 +0200

"Philip K." <philipk <at> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.