unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#12583: latest org-mode changes exposes bug in `comment-forward' in newcomment.el
@ 2012-10-06 11:07 Le Wang
  2012-10-06 12:49 ` Stefan Monnier
  0 siblings, 1 reply; 12+ messages in thread
From: Le Wang @ 2012-10-06 11:07 UTC (permalink / raw)
  To: 12583

Repro with Emacs head:

1. emacs -Q
2. create new org file
3. insert "# blah"
4. C-a
5. M-: (comment-forward 1)

The problem is that `comment-use-syntax' changed from 'undecided to
nil in org.el, which intern exposed a problem in `comment-forward':

It contains:

    (re-search-forward comment-end-skip nil 'move)

But comment-end-skip can validly be nil according to documentation.

-- 
Le





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#12583: latest org-mode changes exposes bug in `comment-forward' in newcomment.el
  2012-10-06 11:07 bug#12583: latest org-mode changes exposes bug in `comment-forward' in newcomment.el Le Wang
@ 2012-10-06 12:49 ` Stefan Monnier
  2012-10-06 15:53   ` bug#12583: latest org-mode changes exposes bug in `comment-forward'in newcomment.el Drew Adams
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2012-10-06 12:49 UTC (permalink / raw)
  To: Le Wang; +Cc: 12583

> 1. emacs -Q
> 2. create new org file
> 3. insert "# blah"
> 4. C-a
> 5. M-: (comment-forward 1)

Before using the comment-* functions, you need to call comment-normalize-vars.


        Stefan





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#12583: latest org-mode changes exposes bug in `comment-forward'in newcomment.el
  2012-10-06 12:49 ` Stefan Monnier
@ 2012-10-06 15:53   ` Drew Adams
  2012-10-06 16:12     ` Stefan Monnier
  2012-10-06 17:07     ` Stefan Monnier
  0 siblings, 2 replies; 12+ messages in thread
From: Drew Adams @ 2012-10-06 15:53 UTC (permalink / raw)
  To: 'Stefan Monnier', 'Le Wang'; +Cc: 12583

> Before using the comment-* functions, you need to call 
> comment-normalize-vars.

Which comment-* functions?  All of them?

Why don't you add a specific guideline about this to the code comments or to
some doc string?

The doc string of `comment-normalize-vars' says, BTW, that it is the functions
that are autoloaded from newcomment.el that should call `c-n-v'.  But you seem
to be saying that those are not the only ones.

It is also the case that not all functions that are autoloaded from
newcomment.el call `c-n-v'.  So what exactly is the proper criterion?  (Or do
some such functions in newcomment.el perhaps need to be corrected to call
`c-n-v'?)

For my own use, I wonder whether I should be calling `c-n-v' before I make use
of `comment-search-forward'.  That function has no autoload cookie, so it does
not fit your prescription, and I have not been calling `c-n-v' and have not
noticed any problem, but I wonder anyway, since you brought this up.  Just when
is it necessary or useful to call `c-n-v'?

BTW, why are there so many things besided commands that have autoload cookies in
newcomment.el?  I know that you are not particularly in favor of unnecessarily
autoloading variables, so I wonder what the story is here.  Thx.






^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#12583: latest org-mode changes exposes bug in `comment-forward'in newcomment.el
  2012-10-06 15:53   ` bug#12583: latest org-mode changes exposes bug in `comment-forward'in newcomment.el Drew Adams
@ 2012-10-06 16:12     ` Stefan Monnier
  2012-10-06 16:37       ` Drew Adams
  2012-10-06 17:07     ` Stefan Monnier
  1 sibling, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2012-10-06 16:12 UTC (permalink / raw)
  To: Drew Adams; +Cc: 12583, 'Le Wang'

> For my own use, I wonder whether I should be calling `c-n-v' before
> I make use of `comment-search-forward'.

Yes, you should.

> BTW, why are there so many things besided commands that have autoload
> cookies in newcomment.el?  I know that you are not particularly in
> favor of unnecessarily autoloading variables, so I wonder what the
> story is here.  Thx.

The functionality of newcomment.el used to be in simple.el and hence
preloaded.  To make the transition as smooth as possible (there were
already plenty of changes) I autoloaded all the things which used to
be preloaded.


        Stefan





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#12583: latest org-mode changes exposes bug in `comment-forward'in newcomment.el
  2012-10-06 16:12     ` Stefan Monnier
@ 2012-10-06 16:37       ` Drew Adams
  2012-10-06 16:48         ` Stefan Monnier
  0 siblings, 1 reply; 12+ messages in thread
From: Drew Adams @ 2012-10-06 16:37 UTC (permalink / raw)
  To: 'Stefan Monnier'; +Cc: 12583, 'Le Wang'

> > For my own use, I wonder whether I should be calling `c-n-v' before
> > I make use of `comment-search-forward'.
> 
> Yes, you should.

OK, thanks.  Please consider providing specific guidelines about this,
somewhere.

> > BTW, why are there so many things besided commands that 
> > have autoload cookies in newcomment.el?  I know that you
> > are not particularly in favor of unnecessarily autoloading
> > variables, so I wonder what the story is here.  Thx.
> 
> The functionality of newcomment.el used to be in simple.el and hence
> preloaded.  To make the transition as smooth as possible (there were
> already plenty of changes) I autoloaded all the things which used to
> be preloaded.

OK, so it was only for a transition.  Presumably that means that some of those
autoloads will be removed at some point.






^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#12583: latest org-mode changes exposes bug in `comment-forward'in newcomment.el
  2012-10-06 16:37       ` Drew Adams
@ 2012-10-06 16:48         ` Stefan Monnier
  2012-10-06 16:57           ` Drew Adams
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2012-10-06 16:48 UTC (permalink / raw)
  To: Drew Adams; +Cc: 12583, 'Le Wang'

>> > For my own use, I wonder whether I should be calling `c-n-v' before
>> > I make use of `comment-search-forward'.
>> Yes, you should.
> OK, thanks.  Please consider providing specific guidelines about this,
> somewhere.

I think the guideline "commands that use comment-* functions should call
c-n-v first" is a good approximation.

> OK, so it was only for a transition.  Presumably that means that some
> of those autoloads will be removed at some point.

Except that I don't know how to mark obsolete the auto-loading of
a variable, so the transition would be just as abrupt now as it would
have been then.
I guess we can remove autoloads one at a time and see when
someone screams.


        Stefan





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#12583: latest org-mode changes exposes bug in `comment-forward'in newcomment.el
  2012-10-06 16:48         ` Stefan Monnier
@ 2012-10-06 16:57           ` Drew Adams
  0 siblings, 0 replies; 12+ messages in thread
From: Drew Adams @ 2012-10-06 16:57 UTC (permalink / raw)
  To: 'Stefan Monnier'; +Cc: 12583, 'Le Wang'

> > OK, thanks.  Please consider providing specific guidelines 
> > about this, somewhere.
> 
> I think the guideline "commands that use comment-* functions 
> should call c-n-v first" is a good approximation.

Then please put that guideline somewhere.  (Currently, the doc string mentions
only autoloaded functions in newcomment.el.)

IOW, I've gotten the message, but this is not obvious, so please help other
users by pointing this out somewhere.

> > OK, so it was only for a transition.  Presumably that means 
> > that some of those autoloads will be removed at some point.
> 
> Except that I don't know how to mark obsolete the auto-loading of
> a variable, so the transition would be just as abrupt now as it would
> have been then.  I guess we can remove autoloads one at a time
> and see when someone screams.

I see.  Is this what is done whenever something that was formerly preloaded gets
moved to a library that is not preloaded?

To be clear, it doesn't bother me that we autoload something that you might
consider extra.  I'm just pointing this out as an apparent exception - a place
where you might want to take a second look (which you've now done).






^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#12583: latest org-mode changes exposes bug in `comment-forward'in newcomment.el
  2012-10-06 15:53   ` bug#12583: latest org-mode changes exposes bug in `comment-forward'in newcomment.el Drew Adams
  2012-10-06 16:12     ` Stefan Monnier
@ 2012-10-06 17:07     ` Stefan Monnier
  2012-10-06 17:12       ` Drew Adams
  1 sibling, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2012-10-06 17:07 UTC (permalink / raw)
  To: Drew Adams; +Cc: 12583, 'Le Wang'

> The doc string of `comment-normalize-vars' says, BTW, that it is the
> functions that are autoloaded from newcomment.el that should call
> `c-n-v'.

That's actually a good guideline, yes.

> But you seem to be saying that those are not the only ones.

No.  AFAICT it's basically the same since all the autoloaded functions
of newcomment.el are commands.

> It is also the case that not all functions that are autoloaded from
> newcomment.el call `c-n-v'.

That would be a bug, I think.


        Stefan





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#12583: latest org-mode changes exposes bug in `comment-forward'in newcomment.el
  2012-10-06 17:07     ` Stefan Monnier
@ 2012-10-06 17:12       ` Drew Adams
  2012-10-07  0:48         ` Stefan Monnier
  0 siblings, 1 reply; 12+ messages in thread
From: Drew Adams @ 2012-10-06 17:12 UTC (permalink / raw)
  To: 'Stefan Monnier'; +Cc: 12583, 'Le Wang'

> > The doc string of `comment-normalize-vars' says, BTW, that it is the
> > functions that are autoloaded from newcomment.el that should call
> > `c-n-v'.
> 
> That's actually a good guideline, yes.

But apparently not sufficient, in terms of helping users with their own uses of
newcomment.el functions.  IOW, it should apparently be about not commands that
use newcomment functions, not just newcomment commands.






^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#12583: latest org-mode changes exposes bug in `comment-forward'in newcomment.el
  2012-10-06 17:12       ` Drew Adams
@ 2012-10-07  0:48         ` Stefan Monnier
  2012-10-07  2:51           ` Drew Adams
  2012-10-23  2:18           ` Chong Yidong
  0 siblings, 2 replies; 12+ messages in thread
From: Stefan Monnier @ 2012-10-07  0:48 UTC (permalink / raw)
  To: Drew Adams; +Cc: 12583, 'Le Wang'

> But apparently not sufficient, in terms of helping users with their
> own uses of newcomment.el functions.  IOW, it should apparently be
> about not commands that use newcomment functions, not just
        ^^^
I assume you meant "all" rather than "not", right?  If so, I agree.

> newcomment commands.


        Stefan





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#12583: latest org-mode changes exposes bug in `comment-forward'in newcomment.el
  2012-10-07  0:48         ` Stefan Monnier
@ 2012-10-07  2:51           ` Drew Adams
  2012-10-23  2:18           ` Chong Yidong
  1 sibling, 0 replies; 12+ messages in thread
From: Drew Adams @ 2012-10-07  2:51 UTC (permalink / raw)
  To: 'Stefan Monnier'; +Cc: 12583, 'Le Wang'

> > But apparently not sufficient, in terms of helping users with their
> > own uses of newcomment.el functions.  IOW, it should apparently be
> > about not commands that use newcomment functions, not just
>         ^^^
> I assume you meant "all" rather than "not", right?  If so, I agree.
> 
> > newcomment commands.

Yes (in fact I meant that that first `not' be removed).  Thx.






^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#12583: latest org-mode changes exposes bug in `comment-forward'in newcomment.el
  2012-10-07  0:48         ` Stefan Monnier
  2012-10-07  2:51           ` Drew Adams
@ 2012-10-23  2:18           ` Chong Yidong
  1 sibling, 0 replies; 12+ messages in thread
From: Chong Yidong @ 2012-10-23  2:18 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12583, 'Le Wang'

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

>> But apparently not sufficient, in terms of helping users with their
>> own uses of newcomment.el functions.  IOW, it should apparently be
>> about not commands that use newcomment functions, not just
>         ^^^
> I assume you meant "all" rather than "not", right?  If so, I agree.

I updated the doc of comment-normalize-vars, and the commentary of
newcomment.el, accordingly.





^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-10-23  2:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-06 11:07 bug#12583: latest org-mode changes exposes bug in `comment-forward' in newcomment.el Le Wang
2012-10-06 12:49 ` Stefan Monnier
2012-10-06 15:53   ` bug#12583: latest org-mode changes exposes bug in `comment-forward'in newcomment.el Drew Adams
2012-10-06 16:12     ` Stefan Monnier
2012-10-06 16:37       ` Drew Adams
2012-10-06 16:48         ` Stefan Monnier
2012-10-06 16:57           ` Drew Adams
2012-10-06 17:07     ` Stefan Monnier
2012-10-06 17:12       ` Drew Adams
2012-10-07  0:48         ` Stefan Monnier
2012-10-07  2:51           ` Drew Adams
2012-10-23  2:18           ` Chong Yidong

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).