* bug#61340: 29.0.60; Extra space in xref buffer
@ 2023-02-07 7:40 Juri Linkov
2023-02-08 0:58 ` Dmitry Gutov
0 siblings, 1 reply; 11+ messages in thread
From: Juri Linkov @ 2023-02-07 7:40 UTC (permalink / raw)
To: 61340
This is a small problem and I didn't notice it until now when
there was an important distinction whether there is a space
character at the beginning of the line, and it was misleading
to see that xref wrongly says there is a leading space.
This is because currently xref output differs from grep and occur
where there is no space between colon and the text from file.
Here is the fix:
```
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index f01b8a1af18..6160f217afb 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -1108,7 +1108,7 @@ xref--insert-xrefs
maximize (xref-location-line
(xref-item-location xref)))
for line-format = (and max-line
- (format "%%%dd: " (1+ (floor (log max-line 10)))))
+ (format "%%%dd:" (1+ (floor (log max-line 10)))))
with item-text-props = (list 'mouse-face 'highlight
'keymap xref--button-map
'help-echo
```
^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#61340: 29.0.60; Extra space in xref buffer
2023-02-07 7:40 bug#61340: 29.0.60; Extra space in xref buffer Juri Linkov
@ 2023-02-08 0:58 ` Dmitry Gutov
2023-02-08 7:35 ` Juri Linkov
0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2023-02-08 0:58 UTC (permalink / raw)
To: Juri Linkov, 61340
Hi!
On 07/02/2023 09:40, Juri Linkov wrote:
> This is a small problem and I didn't notice it until now when
> there was an important distinction whether there is a space
> character at the beginning of the line, and it was misleading
> to see that xref wrongly says there is a leading space.
> This is because currently xref output differs from grep and occur
> where there is no space between colon and the text from file.
> Here is the fix:
>
> ```
> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index f01b8a1af18..6160f217afb 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -1108,7 +1108,7 @@ xref--insert-xrefs
> maximize (xref-location-line
> (xref-item-location xref)))
> for line-format = (and max-line
> - (format "%%%dd: " (1+ (floor (log max-line 10)))))
> + (format "%%%dd:" (1+ (floor (log max-line 10)))))
> with item-text-props = (list 'mouse-face 'highlight
> 'keymap xref--button-map
> 'help-echo
> ```
This was originally an effort to give the outputted text some "breathing
room", and I think it looks a little better.
But if it can cause problems sometimes, I don't mind the proposed
change. After all, both Grep and Occur don't have this space, and
consistency is good.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#61340: 29.0.60; Extra space in xref buffer
2023-02-08 0:58 ` Dmitry Gutov
@ 2023-02-08 7:35 ` Juri Linkov
2023-02-08 15:14 ` Dmitry Gutov
0 siblings, 1 reply; 11+ messages in thread
From: Juri Linkov @ 2023-02-08 7:35 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 61340
close 61340 30.0.50
thanks
>> @@ -1108,7 +1108,7 @@ xref--insert-xrefs
>> maximize (xref-location-line
>> (xref-item-location xref)))
>> for line-format = (and max-line
>> - (format "%%%dd: " (1+ (floor (log max-line 10)))))
>> + (format "%%%dd:" (1+ (floor (log max-line 10)))))
>> with item-text-props = (list 'mouse-face 'highlight
>> 'keymap xref--button-map
>> 'help-echo
>> ```
>
> This was originally an effort to give the outputted text some "breathing
> room", and I think it looks a little better.
Indeed, it looks better but unfortunately at the cost of caused ambiguity.
I guess this is the reason why Grep and Occur don't add space.
> But if it can cause problems sometimes, I don't mind the proposed
> change. After all, both Grep and Occur don't have this space, and
> consistency is good.
Thanks, so now pushed to master.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#61340: 29.0.60; Extra space in xref buffer
2023-02-08 7:35 ` Juri Linkov
@ 2023-02-08 15:14 ` Dmitry Gutov
2023-02-08 18:51 ` Juri Linkov
0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2023-02-08 15:14 UTC (permalink / raw)
To: Juri Linkov; +Cc: 61340
On 08/02/2023 09:35, Juri Linkov wrote:
> close 61340 30.0.50
> thanks
>
>>> @@ -1108,7 +1108,7 @@ xref--insert-xrefs
>>> maximize (xref-location-line
>>> (xref-item-location xref)))
>>> for line-format = (and max-line
>>> - (format "%%%dd: " (1+ (floor (log max-line 10)))))
>>> + (format "%%%dd:" (1+ (floor (log max-line 10)))))
>>> with item-text-props = (list 'mouse-face 'highlight
>>> 'keymap xref--button-map
>>> 'help-echo
>>> ```
>> This was originally an effort to give the outputted text some "breathing
>> room", and I think it looks a little better.
> Indeed, it looks better but unfortunately at the cost of caused ambiguity.
> I guess this is the reason why Grep and Occur don't add space.
>
I suppose we could try to tone down the colon. Occur uses 'shadow' for
both the number and the colon; Grep uses the color from 'default' (with
an underline).
If we fontify it with 'shadow' rather than 'xref-line-number', that
might look a little better.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#61340: 29.0.60; Extra space in xref buffer
2023-02-08 15:14 ` Dmitry Gutov
@ 2023-02-08 18:51 ` Juri Linkov
2023-02-08 22:37 ` Dmitry Gutov
0 siblings, 1 reply; 11+ messages in thread
From: Juri Linkov @ 2023-02-08 18:51 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 61340
>>> This was originally an effort to give the outputted text some "breathing
>>> room", and I think it looks a little better.
>> Indeed, it looks better but unfortunately at the cost of caused ambiguity.
>> I guess this is the reason why Grep and Occur don't add space.
>
> I suppose we could try to tone down the colon. Occur uses 'shadow' for both
> the number and the colon; Grep uses the color from 'default' (with an
> underline).
>
> If we fontify it with 'shadow' rather than 'xref-line-number', that might
> look a little better.
I already forgot that the default color is too glaring since I customized
it long ago to '(xref-line-number ((t (:inherit shadow)))). But I also
customized other xref faces e.g. '(xref-file-header ((t (:extend t
:background "grey90")))), so these faces provide a better look.
OTOH, I'm not sure if the default of xref-line-number can be changed
because on the one hand the purple color from compilation-line-number
is needed in grep to make them more noticeable because the line numbers
are in the middle of the line unlike in occur. But on the other hand
the current theme of xref buffers matches all colors of grep, so any
change will make their default themes different.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#61340: 29.0.60; Extra space in xref buffer
2023-02-08 18:51 ` Juri Linkov
@ 2023-02-08 22:37 ` Dmitry Gutov
2023-02-09 17:51 ` Juri Linkov
0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2023-02-08 22:37 UTC (permalink / raw)
To: Juri Linkov; +Cc: 61340
On 08/02/2023 20:51, Juri Linkov wrote:
>>>> This was originally an effort to give the outputted text some "breathing
>>>> room", and I think it looks a little better.
>>> Indeed, it looks better but unfortunately at the cost of caused ambiguity.
>>> I guess this is the reason why Grep and Occur don't add space.
>> I suppose we could try to tone down the colon. Occur uses 'shadow' for both
>> the number and the colon; Grep uses the color from 'default' (with an
>> underline).
>>
>> If we fontify it with 'shadow' rather than 'xref-line-number', that might
>> look a little better.
> I already forgot that the default color is too glaring since I customized
> it long ago to '(xref-line-number ((t (:inherit shadow)))). But I also
> customized other xref faces e.g. '(xref-file-header ((t (:extend t
> :background "grey90")))), so these faces provide a better look.
> OTOH, I'm not sure if the default of xref-line-number can be changed
> because on the one hand the purple color from compilation-line-number
> is needed in grep to make them more noticeable because the line numbers
> are in the middle of the line unlike in occur. But on the other hand
> the current theme of xref buffers matches all colors of grep, so any
> change will make their default themes different.
Here's the change I was suggesting.
The difference is subtle, but seems like an improvement:
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 63e065e696e..581eda0513e 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -1126,7 +1126,9 @@ xref--insert-xrefs
maximize (xref-location-line
(xref-item-location xref)))
for line-format = (and max-line
- (format "%%%dd:" (1+ (floor (log
max-line 10)))))
+ (format
+ #("%%%dd:" 0 4 (face
xref-line-number) 5 6 (face shadow))
+ (1+ (floor (log max-line 10)))))
with item-text-props = (list 'mouse-face 'highlight
'keymap xref--button-map
'help-echo
@@ -1146,8 +1148,7 @@ xref--insert-xrefs
((and (equal line prev-line)
(equal prev-group group))
"")
- (t (propertize (format line-format line)
- 'face 'xref-line-number)))))
+ (t (format line-format line)))))
;; Render multiple matches on the same line, together.
(when (and (equal prev-group group)
(or (null line)
^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#61340: 29.0.60; Extra space in xref buffer
2023-02-08 22:37 ` Dmitry Gutov
@ 2023-02-09 17:51 ` Juri Linkov
2023-02-09 19:58 ` Dmitry Gutov
0 siblings, 1 reply; 11+ messages in thread
From: Juri Linkov @ 2023-02-09 17:51 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 61340
> Here's the change I was suggesting.
>
> The difference is subtle, but seems like an improvement:
>
> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index 63e065e696e..581eda0513e 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -1126,7 +1126,9 @@ xref--insert-xrefs
> maximize (xref-location-line
> (xref-item-location xref)))
> for line-format = (and max-line
> - (format "%%%dd:" (1+ (floor (log
> max-line 10)))))
> + (format
> + #("%%%dd:" 0 4 (face xref-line-number)
> 5 6 (face shadow))
Thanks, this looks nice and closer to the grep output fontification.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#61340: 29.0.60; Extra space in xref buffer
2023-02-09 17:51 ` Juri Linkov
@ 2023-02-09 19:58 ` Dmitry Gutov
2023-02-18 22:04 ` Dmitry Gutov
0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2023-02-09 19:58 UTC (permalink / raw)
To: Juri Linkov; +Cc: 61340
On 09/02/2023 19:51, Juri Linkov wrote:
>> Here's the change I was suggesting.
>>
>> The difference is subtle, but seems like an improvement:
>>
>> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
>> index 63e065e696e..581eda0513e 100644
>> --- a/lisp/progmodes/xref.el
>> +++ b/lisp/progmodes/xref.el
>> @@ -1126,7 +1126,9 @@ xref--insert-xrefs
>> maximize (xref-location-line
>> (xref-item-location xref)))
>> for line-format = (and max-line
>> - (format "%%%dd:" (1+ (floor (log
>> max-line 10)))))
>> + (format
>> + #("%%%dd:" 0 4 (face xref-line-number)
>> 5 6 (face shadow))
> Thanks, this looks nice and closer to the grep output fontification.
Thanks for testing, pushed to master.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#61340: 29.0.60; Extra space in xref buffer
2023-02-09 19:58 ` Dmitry Gutov
@ 2023-02-18 22:04 ` Dmitry Gutov
2023-02-19 6:20 ` Eli Zaretskii
0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2023-02-18 22:04 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 61340, Juri Linkov
Hi Eli,
Should we backport this to emacs-29? The improvement is minor, but the
risk is very small as well.
On 09/02/2023 21:58, Dmitry Gutov wrote:
> On 09/02/2023 19:51, Juri Linkov wrote:
>>> Here's the change I was suggesting.
>>>
>>> The difference is subtle, but seems like an improvement:
>>>
>>> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
>>> index 63e065e696e..581eda0513e 100644
>>> --- a/lisp/progmodes/xref.el
>>> +++ b/lisp/progmodes/xref.el
>>> @@ -1126,7 +1126,9 @@ xref--insert-xrefs
>>> maximize (xref-location-line
>>> (xref-item-location
>>> xref)))
>>> for line-format = (and max-line
>>> - (format "%%%dd:" (1+ (floor (log
>>> max-line 10)))))
>>> + (format
>>> + #("%%%dd:" 0 4 (face
>>> xref-line-number)
>>> 5 6 (face shadow))
>> Thanks, this looks nice and closer to the grep output fontification.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#61340: 29.0.60; Extra space in xref buffer
2023-02-18 22:04 ` Dmitry Gutov
@ 2023-02-19 6:20 ` Eli Zaretskii
2023-02-19 12:27 ` Dmitry Gutov
0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2023-02-19 6:20 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 61340, juri
> Date: Sun, 19 Feb 2023 00:04:05 +0200
> From: Dmitry Gutov <dgutov@yandex.ru>
> Cc: 61340@debbugs.gnu.org, Juri Linkov <juri@linkov.net>
>
> Should we backport this to emacs-29? The improvement is minor, but the
> risk is very small as well.
Fine by me, thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#61340: 29.0.60; Extra space in xref buffer
2023-02-19 6:20 ` Eli Zaretskii
@ 2023-02-19 12:27 ` Dmitry Gutov
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Gutov @ 2023-02-19 12:27 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 61340, juri
On 19/02/2023 08:20, Eli Zaretskii wrote:
>> Date: Sun, 19 Feb 2023 00:04:05 +0200
>> From: Dmitry Gutov<dgutov@yandex.ru>
>> Cc:61340@debbugs.gnu.org, Juri Linkov<juri@linkov.net>
>>
>> Should we backport this to emacs-29? The improvement is minor, but the
>> risk is very small as well.
> Fine by me, thanks.
Thank you, done.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-02-19 12:27 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-07 7:40 bug#61340: 29.0.60; Extra space in xref buffer Juri Linkov
2023-02-08 0:58 ` Dmitry Gutov
2023-02-08 7:35 ` Juri Linkov
2023-02-08 15:14 ` Dmitry Gutov
2023-02-08 18:51 ` Juri Linkov
2023-02-08 22:37 ` Dmitry Gutov
2023-02-09 17:51 ` Juri Linkov
2023-02-09 19:58 ` Dmitry Gutov
2023-02-18 22:04 ` Dmitry Gutov
2023-02-19 6:20 ` Eli Zaretskii
2023-02-19 12:27 ` Dmitry Gutov
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).