gnus/html2text.el

gnus/html2text.el

[Luc Teirlinck]

gnus/html2text.el calls `beginning-of-buffer', disguised as
`html2text-buffer-head' at several places _inside a loop_.
`beginning-of-buffer' should usually not be called from Lisp to begin
with, unless one really _wants_ to set the mark, but calling it
inside a loop completely ruins the mark ring (and probably slows
things down a lot too).  It should use (goto-char (point-min)), as
clearly explained in the beginning-of-buffer docstring.

Unfortunately, after grepping around a little bit, it would at first
view seem that such abuse of {beginning,end}-of-buffer and other mark
setting commands is even substantially more widespread than the kind
of abuses of `interactive-p' we have been looking at.  Whether
something is an abuse or not is not always obvious, because
occasionally one _really_ wants to set the mark from Lisp.
In the case of gnus/html2text.el it _is_ obvious, however.

Sincerely,

Luc.

Re: gnus/html2text.el

[Reiner Steib]

On Mon, Oct 25 2004, Luc Teirlinck wrote:

> gnus/html2text.el calls `beginning-of-buffer', disguised as
> `html2text-buffer-head' at several places _inside a loop_.
> `beginning-of-buffer' should usually not be called from Lisp to begin
> with, unless one really _wants_ to set the mark, but calling it
> inside a loop completely ruins the mark ring (and probably slows
> things down a lot too).  It should use (goto-char (point-min)), as
> clearly explained in the beginning-of-buffer docstring.

Fixed in Gnus v5-10 branch.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: gnus/html2text.el

[Richard Stallman]

    Unfortunately, after grepping around a little bit, it would at first
    view seem that such abuse of {beginning,end}-of-buffer and other mark
    setting commands is even substantially more widespread than the kind
    of abuses of `interactive-p' we have been looking at.

Perhaps we should arrange for byte-compile to warn about use
of these functions from Lisp code.  What do people think of that?

Aside from beginning-of-buffer and end-of-buffer, what functions
are you talking about?

Re: gnus/html2text.el

[Stefan Monnier]

> Perhaps we should arrange for byte-compile to warn about use
> of these functions from Lisp code.  What do people think of that?

That sounds like a good idea.  We need a way to cleanly turn off the
message, but for end-of-buffer we can use (call-interactively
'end-of-buffer).

> Aside from beginning-of-buffer and end-of-buffer, what functions
> are you talking about?

There's things like `replace-regexp', maybe even `switch-to-buffer' (tho
pop-to-buffer should be extended a bit to provide a true substitute for
switch-to-buffer).


        Stefan

Re: gnus/html2text.el

[Kim F. Storm]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Perhaps we should arrange for byte-compile to warn about use
>> of these functions from Lisp code.  What do people think of that?
>
> That sounds like a good idea.  We need a way to cleanly turn off the
> message, but for end-of-buffer we can use (call-interactively
> 'end-of-buffer).


 (with-no-warnings (end-of-buffer))


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: gnus/html2text.el

[Stefan Monnier]

>>> Perhaps we should arrange for byte-compile to warn about use
>>> of these functions from Lisp code.  What do people think of that?
>> 
>> That sounds like a good idea.  We need a way to cleanly turn off the
>> message, but for end-of-buffer we can use (call-interactively
>> 'end-of-buffer).

>  (with-no-warnings (end-of-buffer))

OK, that's a possibility.  It would be better to have something more
specific so that it is clear what is happening.  Specificity becomes
relevant in cases like:

   (with-no-warnings (ent-of-buffer))

which will not give you the warning you'd want to get.
Maybe `with-no-warnings' should take an extra argument listing the kind of
warning to ignore.


        Stefan

Re: gnus/html2text.el

[Luc Teirlinck]

Stefan Monnier wrote:

      (with-no-warnings (ent-of-buffer))

   which will not give you the warning you'd want to get.
   Maybe `with-no-warnings' should take an extra argument listing the kind of
   warning to ignore.

When I use `with-no-warnings', I first compile without the
`with-no-warnings', to see which warnings I get, then I put in the
form and compile again.

Sincerely,

Luc.

Re: gnus/html2text.el

[Miles Bader]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
> Maybe `with-no-warnings' should take an extra argument listing the kind of
> warning to ignore.

Well `with-no-warnings' is a progn-alike, so some care would be needed
to do so in an compatible manner (can't just add a 2nd argument), but it
seems like a cool idea.

Perhaps something like:

   (with-no-warnings :kind interactive-command
      ...)

?

[presumably :kind could be followed by a list or warning names.]

-Miles
-- 
Love is a snowmobile racing across the tundra.  Suddenly it flips over,
pinning you underneath.  At night the ice weasels come.  --Nietzsche

Re: gnus/html2text.el

[Luc Teirlinck]

Richard Stallman wrote:

   Aside from beginning-of-buffer and end-of-buffer, what functions
   are you talking about?

I believe that I remember having seen warnings similar to the ones in
the {beginning,end}-of-buffer docstrings in the docstrings of some
other functions.  But I do not remember which ones they were.

I always do `C-h f' on every function I use and of which I am not
completely sure that I know every single detail about it. 

Sincerely,

Luc.

Re: gnus/html2text.el

[Luc Teirlinck]

Richard Stallman wrote:

   Aside from beginning-of-buffer and end-of-buffer, what functions
   are you talking about?

I now remember one: insert-file.

`(elisp)Coding Conventions' mentions  `replace-string', `replace-regexp'.

Stefan already mentioned the latter.

I guess there must be others.

Sincerely,

Luc.

Re: gnus/html2text.el

[Stefan Monnier]

> When I use `with-no-warnings', I first compile without the
> `with-no-warnings', to see which warnings I get, then I put in the form
> and compile again.

Nice theory.  And next thing you know reality strikes:
when you modify code do you first remove all "with-no-warnings", recompile,
check the output and then put the with-no-warnings back in?

If you could ensure that the code within with-no-warnings was never gonna
change (and that the functions/macros it uses will never again be changed
either), it would be OK, but that's not how things actually happen.


        Stefan

Re: gnus/html2text.el

[Luc Teirlinck]

Stefan Monnier wrote:

   Nice theory.  And next thing you know reality strikes:
   when you modify code do you first remove all "with-no-warnings", recompile,
   check the output and then put the with-no-warnings back in?

Depends.  If the code change is trivial, I would just be extra careful
inside the `with-no-warnings'.  If it is extensive, I would indeed
temporarily remove the `with-no-warnings'.  But the latter should very
rarely happen: `with-no-warnings' is not supposed to cover a large
chunk of code anyway.

How substantive a change is one going to make to
`(with-no-warnings (beginning-of-buffer))'?

I believe that a much bigger problem is that many people do not pay
any attention to compiler warnings.

If one runs `make bootstrap' one gets an endless litany of one
compiler warning after the other, that no one apparently ever paid any
attention to.

Sincerely,

Luc.

Re: gnus/html2text.el

[Luc Teirlinck]

>From my previous message:

   If one runs `make bootstrap' one gets an endless litany of one
   compiler warning after the other, that no one apparently ever paid any
   attention to.

Well, after first running `make maintainer-clean'.  (Unless that is no
longer necessary.)

Sincerely,

Luc.

Re: gnus/html2text.el

[Richard Stallman]

      Specificity becomes
    relevant in cases like:

       (with-no-warnings (ent-of-buffer))

    which will not give you the warning you'd want to get.
    Maybe `with-no-warnings' should take an extra argument listing the kind of
    warning to ignore.

Theoretically this is an issue, but in practice it's not a big
enough problem to be worth adding that feature.  Please let's let
this drop.