* bug#10868: 24.0.93; `comment-style' needs to be explained
@ 2012-02-22 17:17 Drew Adams
2012-02-22 17:38 ` Glenn Morris
0 siblings, 1 reply; 15+ messages in thread
From: Drew Adams @ 2012-02-22 17:17 UTC (permalink / raw)
To: 10868
This is a followup to bug #3370, which was closed without attention to
the bug thread. The problem is that variable `comment-style' is not
explained. Please do so.
See this message from the bug #3370 thread, as well as other parts of
the thread, for a discussion of the problem:
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=3370#88
In GNU Emacs 24.0.93.1 (i386-mingw-nt5.1.2600)
of 2012-02-15 on MARVIN
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
`configure --with-gcc (4.6) --no-opt --enable-checking --cflags
-ID:/devel/emacs/libs/libXpm-3.5.8/include
-ID:/devel/emacs/libs/libXpm-3.5.8/src
-ID:/devel/emacs/libs/libpng-dev_1.4.3-1/include
-ID:/devel/emacs/libs/zlib-dev_1.2.5-2/include
-ID:/devel/emacs/libs/giflib-4.1.4-1/include
-ID:/devel/emacs/libs/jpeg-6b-4/include
-ID:/devel/emacs/libs/tiff-3.8.2-1/include
-ID:/devel/emacs/libs/gnutls-3.0.9/include'
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#10868: 24.0.93; `comment-style' needs to be explained
2012-02-22 17:17 bug#10868: 24.0.93; `comment-style' needs to be explained Drew Adams
@ 2012-02-22 17:38 ` Glenn Morris
2012-02-22 18:06 ` Drew Adams
0 siblings, 1 reply; 15+ messages in thread
From: Glenn Morris @ 2012-02-22 17:38 UTC (permalink / raw)
To: 10868
> The problem is that variable `comment-style' is not explained.
comment-style is a variable defined in `newcomment.el'.
Its value is indent
Documentation:
Style to be used for `comment-region'.
See `comment-styles' for a list of available styles.
comment-styles is a variable defined in `newcomment.el'.
[...]
((plain nil nil nil nil "Start in column 0 (do not indent), as in Emacs-20")
(indent-or-triple nil nil nil multi-char "Start in column 0, but only for single-char starters")
(indent nil nil nil t "Full comment per line, ends not aligned")
(aligned nil t nil t "Full comment per line, ends aligned")
(box nil t t t "Full comment per line, ends aligned, + top and bottom")
(extra-line t nil t t "One comment for all lines, end on a line by itself")
(multi-line t nil nil t "One comment for all lines, end on last commented line")
(box-multi t t t t "One comment for all lines, + top and bottom"))
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#10868: 24.0.93; `comment-style' needs to be explained
2012-02-22 17:38 ` Glenn Morris
@ 2012-02-22 18:06 ` Drew Adams
2012-02-22 18:11 ` Glenn Morris
0 siblings, 1 reply; 15+ messages in thread
From: Drew Adams @ 2012-02-22 18:06 UTC (permalink / raw)
To: 'Glenn Morris', 10868
> > The problem is that variable `comment-style' is not explained.
>
> comment-style is a variable defined in `newcomment.el'.
> Its value is indent
>
> Documentation:
> Style to be used for `comment-region'.
> See `comment-styles' for a list of available styles.
>
> comment-styles is a variable defined in `newcomment.el'.
> [...]
> ((plain nil nil nil nil "Start in column 0 (do not indent),
> as in Emacs-20")
> (indent-or-triple nil nil nil multi-char "Start in column 0,
> but only for single-char starters")
> (indent nil nil nil t "Full comment per line, ends not aligned")
> (aligned nil t nil t "Full comment per line, ends aligned")
> (box nil t t t "Full comment per line, ends aligned, + top
> and bottom")
> (extra-line t nil t t "One comment for all lines, end on a
> line by itself")
> (multi-line t nil nil t "One comment for all lines, end on
> last commented line")
> (box-multi t t t t "One comment for all lines, + top and bottom"))
You are just parroting the existing, insufficient doc.
Read the bug #3370 thread, if you want to understand the problem. There is no
_explanation_ of `comment-style'.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#10868: 24.0.93; `comment-style' needs to be explained
2012-02-22 18:06 ` Drew Adams
@ 2012-02-22 18:11 ` Glenn Morris
2012-02-22 19:01 ` Drew Adams
0 siblings, 1 reply; 15+ messages in thread
From: Glenn Morris @ 2012-02-22 18:11 UTC (permalink / raw)
To: 10868
> Read the bug #3370 thread, if you want to understand the problem.
Unfortunately, I did. It is long-winded and goes nowhere.
I encourage everyone else not to bother.
> There is no _explanation_ of `comment-style'.
Right, well, good luck with your future bug reporting.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#10868: 24.0.93; `comment-style' needs to be explained
2012-02-22 18:11 ` Glenn Morris
@ 2012-02-22 19:01 ` Drew Adams
2012-02-23 9:22 ` Lawrence Mitchell
0 siblings, 1 reply; 15+ messages in thread
From: Drew Adams @ 2012-02-22 19:01 UTC (permalink / raw)
To: 'Glenn Morris', 10868
> > Read the bug #3370 thread, if you want to understand the problem.
>
> Unfortunately, I did. It is long-winded and goes nowhere.
> I encourage everyone else not to bother.
Is it really so hard to understand?
"the doc string for `comment-style' (which is pretty much
useless) refers to `comment-styles', which is not
recognized (and therefore has no link)."
emacs -Q
C-h v comment-style
Style to be used for `comment-region'.
See `comment-styles' for a list of available styles.
C-h v comment-styles
Bzzzzt -not recognized - no such variable, no such "list of available styles"
... until you load newcomment.el.
There is a user option (`comment-style'), which you can customize. But its doc
tells you nothing useful. It just refers you to an internal variable
(`comment-styles'), which has no doc until you load its library.
The doc for `comment-styles' _is_ helpful ... once it becomes available.
Here is a case where we actually autoload a user option, `comment-style', so its
(useless) doc is available prior to loading the library.
But that doc string just refers you to a variable (constant, actually) whose doc
(the advertised "list of available styles") is not available until you load the
library.
Bravo. Thanks for helping users out.
> > There is no _explanation_ of `comment-style'.
>
> Right, well, good luck with your future bug reporting.
So constructive.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#10868: 24.0.93; `comment-style' needs to be explained
2012-02-22 19:01 ` Drew Adams
@ 2012-02-23 9:22 ` Lawrence Mitchell
2012-02-23 14:31 ` Stefan Monnier
2012-02-23 17:16 ` Glenn Morris
0 siblings, 2 replies; 15+ messages in thread
From: Lawrence Mitchell @ 2012-02-23 9:22 UTC (permalink / raw)
To: 10868
Drew Adams wrote:
>>> Read the bug #3370 thread, if you want to understand the problem.
>> Unfortunately, I did. It is long-winded and goes nowhere.
>> I encourage everyone else not to bother.
> Is it really so hard to understand?
> "the doc string for `comment-style' (which is pretty much
> useless) refers to `comment-styles', which is not
> recognized (and therefore has no link)."
> emacs -Q
> C-h v comment-style
> Style to be used for `comment-region'.
> See `comment-styles' for a list of available styles.
> C-h v comment-styles
> Bzzzzt -not recognized - no such variable, no such "list of available styles"
> ... until you load newcomment.el.
[...]
OK, here's the problem, and here's a patch.
2012-02-23 Lawrence Mitchell <wence@gmx.li>
* lisp/newcomment.el: Autoload comment-styles (Bug#3370).
The docstring of comment-style refers the user to comment-styles for
details on what the values mean. However, this is not available until
after newcomment.el is loaded. So autoload comment-styles to avoid
this problem.
---
| 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
--git a/lisp/newcomment.el b/lisp/newcomment.el
index 16282af..cd887cb 100644
--- a/lisp/newcomment.el
+++ b/lisp/newcomment.el
@@ -185,6 +185,7 @@ The `plain' comment style doubles this value.
This should generally stay 0, except for a few modes like Lisp where
it is 1 so that regions are commented with two or three semi-colons.")
+;;;###autoload
(defconst comment-styles
'((plain nil nil nil nil
"Start in column 0 (do not indent), as in Emacs-20")
--
1.7.9.rc0.23.g7e521
^ permalink raw reply related [flat|nested] 15+ messages in thread* bug#10868: 24.0.93; `comment-style' needs to be explained
2012-02-23 9:22 ` Lawrence Mitchell
@ 2012-02-23 14:31 ` Stefan Monnier
2012-02-23 15:35 ` Drew Adams
2012-02-25 3:08 ` Chong Yidong
2012-02-23 17:16 ` Glenn Morris
1 sibling, 2 replies; 15+ messages in thread
From: Stefan Monnier @ 2012-02-23 14:31 UTC (permalink / raw)
To: Lawrence Mitchell; +Cc: 10868
> OK, here's the problem, and here's a patch.
> 2012-02-23 Lawrence Mitchell <wence@gmx.li>
> * lisp/newcomment.el: Autoload comment-styles (Bug#3370).
Another fix is to remove the ;;;###autoload from comment-style.
If that doesn't introduce other problems, it's a better solution.
Of course, yet another solution is to preload newcomment.el (an option
that was rejected back when newcomment.el was introduced into Emacs-21).
Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread* bug#10868: 24.0.93; `comment-style' needs to be explained
2012-02-23 14:31 ` Stefan Monnier
@ 2012-02-23 15:35 ` Drew Adams
2012-02-23 16:04 ` Stefan Monnier
2012-02-25 3:08 ` Chong Yidong
1 sibling, 1 reply; 15+ messages in thread
From: Drew Adams @ 2012-02-23 15:35 UTC (permalink / raw)
To: 'Stefan Monnier', 'Lawrence Mitchell'; +Cc: 10868
> > OK, here's the problem, and here's a patch.
> > 2012-02-23 Lawrence Mitchell <wence@gmx.li>
> > * lisp/newcomment.el: Autoload comment-styles (Bug#3370).
>
> Another fix is to remove the ;;;###autoload from comment-style.
> If that doesn't introduce other problems, it's a better solution.
>
> Of course, yet another solution is to preload newcomment.el (an option
> that was rejected back when newcomment.el was introduced into
> Emacs-21).
FWIW, I believe I'm OK with any of the 3 proposed solutions.
Haven't thought about it much, but they all seem OK.
Typically, Stefan, you do not like `;;;##autoload' cookies for user options (at
least this has been argued before). Is there a reason here why you might prefer
one of the other solutions? What "other problems" are you thinking of?
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#10868: 24.0.93; `comment-style' needs to be explained
2012-02-23 15:35 ` Drew Adams
@ 2012-02-23 16:04 ` Stefan Monnier
2012-02-23 16:13 ` Drew Adams
0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2012-02-23 16:04 UTC (permalink / raw)
To: Drew Adams; +Cc: 10868, 'Lawrence Mitchell'
> Typically, Stefan, you do not like `;;;##autoload' cookies for user
> options (at least this has been argued before). Is there a reason
> here why you might prefer one of the other solutions?
I like preloading newcomment.el. It's a package that is very likely to
be used in most Emacs sessions, and it has a whole bunch of autoload
cookies in it (and prior to introducing newcomment.el, the corresponding
functionality was preloaded since it was in simple.el).
> What "other problems" are you thinking of?
The ones I can't think of ;-)
Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread* bug#10868: 24.0.93; `comment-style' needs to be explained
2012-02-23 16:04 ` Stefan Monnier
@ 2012-02-23 16:13 ` Drew Adams
0 siblings, 0 replies; 15+ messages in thread
From: Drew Adams @ 2012-02-23 16:13 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 10868, 'Lawrence Mitchell'
> I like preloading newcomment.el. It's a package that is very
> likely to be used in most Emacs sessions, and it has a whole
> bunch of autoload cookies in it (and prior to introducing
> newcomment.el, the corresponding functionality was preloaded
> since it was in simple.el).
Sounds good to me. And it's good to know those (3 strong) reasons, for future
contexts.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#10868: 24.0.93; `comment-style' needs to be explained
2012-02-23 14:31 ` Stefan Monnier
2012-02-23 15:35 ` Drew Adams
@ 2012-02-25 3:08 ` Chong Yidong
1 sibling, 0 replies; 15+ messages in thread
From: Chong Yidong @ 2012-02-25 3:08 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 10868, Lawrence Mitchell
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> OK, here's the problem, and here's a patch.
>> 2012-02-23 Lawrence Mitchell <wence@gmx.li>
>> * lisp/newcomment.el: Autoload comment-styles (Bug#3370).
>
> Another fix is to remove the ;;;###autoload from comment-style.
> If that doesn't introduce other problems, it's a better solution.
The docstring of comment-region refers to comment-style, so removing the
autoload of comment-style brings us back to square one.
I added an autoload for comment-styles.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#10868: 24.0.93; `comment-style' needs to be explained
2012-02-23 9:22 ` Lawrence Mitchell
2012-02-23 14:31 ` Stefan Monnier
@ 2012-02-23 17:16 ` Glenn Morris
2012-02-23 17:22 ` Glenn Morris
` (2 more replies)
1 sibling, 3 replies; 15+ messages in thread
From: Glenn Morris @ 2012-02-23 17:16 UTC (permalink / raw)
To: Lawrence Mitchell; +Cc: 10868
Lawrence Mitchell wrote:
> The docstring of comment-style refers the user to comment-styles for
> details on what the values mean. However, this is not available until
> after newcomment.el is loaded. So autoload comment-styles to avoid
> this problem.
Then this is an instance of a general problem. See eg
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=1768
for the same thing.
IMO there should be a general solution rather than papering over every
instance that occurs with more pre/autoloading.
^ permalink raw reply [flat|nested] 15+ messages in thread* bug#10868: 24.0.93; `comment-style' needs to be explained
2012-02-23 17:16 ` Glenn Morris
@ 2012-02-23 17:22 ` Glenn Morris
2012-02-23 17:24 ` Lawrence Mitchell
2012-02-23 17:34 ` Drew Adams
2 siblings, 0 replies; 15+ messages in thread
From: Glenn Morris @ 2012-02-23 17:22 UTC (permalink / raw)
To: 10868
Glenn Morris wrote:
> Then this is an instance of a general problem. See eg
PS Someone might want to rename this report to reflect the real issue.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#10868: 24.0.93; `comment-style' needs to be explained
2012-02-23 17:16 ` Glenn Morris
2012-02-23 17:22 ` Glenn Morris
@ 2012-02-23 17:24 ` Lawrence Mitchell
2012-02-23 17:34 ` Drew Adams
2 siblings, 0 replies; 15+ messages in thread
From: Lawrence Mitchell @ 2012-02-23 17:24 UTC (permalink / raw)
To: 10868
Glenn Morris wrote:
> Lawrence Mitchell wrote:
>> The docstring of comment-style refers the user to comment-styles for
>> details on what the values mean. However, this is not available until
>> after newcomment.el is loaded. So autoload comment-styles to avoid
>> this problem.
> Then this is an instance of a general problem. See eg
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=1768
> for the same thing.
> IMO there should be a general solution rather than papering over every
> instance that occurs with more pre/autoloading.
Sure, but you can't do this, right? You don't know where the
variable is defined until /after/ the file it's defined in is
loaded. Sure, you can guess that links in a docstring for a
variable that is autoloaded from package foo may also lead to
other variables in package foo, but that's not foolproof.
I guess the help system could load the file of autoloaded
variables when showing their docstring, but people are bound to
complain about that too.
Lawrence
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#10868: 24.0.93; `comment-style' needs to be explained
2012-02-23 17:16 ` Glenn Morris
2012-02-23 17:22 ` Glenn Morris
2012-02-23 17:24 ` Lawrence Mitchell
@ 2012-02-23 17:34 ` Drew Adams
2 siblings, 0 replies; 15+ messages in thread
From: Drew Adams @ 2012-02-23 17:34 UTC (permalink / raw)
To: 'Glenn Morris', 'Lawrence Mitchell'; +Cc: 10868
> > The docstring of comment-style refers the user to comment-styles for
> > details on what the values mean. However, this is not available
> > until after newcomment.el is loaded. So autoload comment-styles
> > to avoid this problem.
>
> Then this is an instance of a general problem. See eg
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=1768
> for the same thing.
>
> IMO there should be a general solution rather than papering over every
> instance that occurs with more pre/autoloading.
Maybe, maybe not.
No one has suggested "papering over *every instance* that occurs with more
pre/autoloading". Quite the contrary. Stefan gave some specific guidelines
(reasons) why preloading makes sense in this _particular_ case.
But if what you say is true and is agreed upon, we can discuss the general
problem & solution separately - in emacs-devel, for example.
Wrt _this_ bug report: let us not let the perfect become the enemy of the good.
Let's not wait for some expected or hoped-for general solution before fixing
this particular bug.
It's hard enough to get approval that a particular option's doc string be
preloaded. Let's not throw abstract obstacles in the path, once that need has
been recognized in a particular case.
> PS Someone might want to rename this report to reflect the real issue.
Please, no. Please do not try to detour this bug fix.
Instead, file a separate bug report for the general case that you suppose. Or
better yet, bring up the question for discussion at emacs-devel.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2012-02-25 3:08 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-22 17:17 bug#10868: 24.0.93; `comment-style' needs to be explained Drew Adams
2012-02-22 17:38 ` Glenn Morris
2012-02-22 18:06 ` Drew Adams
2012-02-22 18:11 ` Glenn Morris
2012-02-22 19:01 ` Drew Adams
2012-02-23 9:22 ` Lawrence Mitchell
2012-02-23 14:31 ` Stefan Monnier
2012-02-23 15:35 ` Drew Adams
2012-02-23 16:04 ` Stefan Monnier
2012-02-23 16:13 ` Drew Adams
2012-02-25 3:08 ` Chong Yidong
2012-02-23 17:16 ` Glenn Morris
2012-02-23 17:22 ` Glenn Morris
2012-02-23 17:24 ` Lawrence Mitchell
2012-02-23 17:34 ` Drew Adams
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).