Re: [gnus git] branch master updated: Make gnus-group-add-icon work

Re: [gnus git] branch master updated: Make gnus-group-add-icon work

[Lars Magne Ingebrigtsen]

Julien Danjou <julien@danjou.info> writes:

> +(defcustom gnus-group-update-hook '(gnus-group-highlight-line gnus-group-add-icon)

I don't think the icon thing should be added to the hook by default...

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: [gnus git] branch master updated: Make gnus-group-add-icon work

[Julien Danjou]

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

On Wed, Sep 22 2010, Lars Magne Ingebrigtsen wrote:

> I don't think the icon thing should be added to the hook by default...

Why? It does nothing if %E is not used in format, but will not work if
%E is used.

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [gnus git] branch master updated: Make gnus-group-add-icon work

[Lars Magne Ingebrigtsen]

Julien Danjou <julien@danjou.info> writes:

>> I don't think the icon thing should be added to the hook by default...
>
> Why? It does nothing if %E is not used in format, but will not work if
> %E is used.

Hooks are generally for user-enabled stuff.  Even the highlight thing
really shouldn't be in that hook.

And it looks like an expensive function.  It starts with:

  (save-excursion
    (let* ((end (line-end-position))
           ;; now find out where the line starts and leave point there.
           (beg (line-beginning-position)))
      (save-restriction
        (narrow-to-region beg end)
        (goto-char beg)
        (let ((mystart (text-property-any beg end 'gnus-group-icon t)))

And this is run once per line, whether you have %E to or not, isn't it?

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: [gnus git] branch master updated: Make gnus-group-add-icon work

[Julien Danjou]

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

On Wed, Sep 22 2010, Lars Magne Ingebrigtsen wrote:

> Hooks are generally for user-enabled stuff.  Even the highlight thing
> really shouldn't be in that hook.

I agree. But it was not empty! This is why I did add it here. :-)

OTOH I can look to empty the hook by default.

> And it looks like an expensive function.  It starts with:
>
>   (save-excursion
>     (let* ((end (line-end-position))
>            ;; now find out where the line starts and leave point there.
>            (beg (line-beginning-position)))
>       (save-restriction
>         (narrow-to-region beg end)
>         (goto-char beg)
>         (let ((mystart (text-property-any beg end 'gnus-group-icon t)))
>
> And this is run once per line, whether you have %E to or not, isn't it?

Sure, it is. But I will say early optimization is bad. I don't think is
is slowing things down. C'mon! ;-)

But, what I would prefer to do, is to build the %E value with the image
directly, and then use that propertized string to replace %E in the
spec. This would avoid scanning group lines…

What do you think?

PS: You know, I'm new, and I don't think like I'm authorized to move
that old code all around, so I'm doing things little by little for now.
;-)

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [gnus git] branch master updated: Make gnus-group-add-icon work

[Lars Magne Ingebrigtsen]

Julien Danjou <julien@danjou.info> writes:

> I agree. But it was not empty! This is why I did add it here. :-)
>
> OTOH I can look to empty the hook by default.

Putting stuff in the hook was a bad decision, but it's there, so...

> But, what I would prefer to do, is to build the %E value with the image
> directly, and then use that propertized string to replace %E in the
> spec. This would avoid scanning group lines…
>
> What do you think?

Yes, that sounds good.  It'd be cleaner and not affect people who don't
use %E.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen