all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* 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 external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.