unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#8638: 24.0.50; Imenu should not include vacuous defvars
@ 2011-05-08 18:15 Drew Adams
  2011-05-08 18:50 ` Juanma Barranquero
  0 siblings, 1 reply; 19+ messages in thread
From: Drew Adams @ 2011-05-08 18:15 UTC (permalink / raw)
  To: 8638

Could we please improve `lisp-imenu-generic-expression so that it does
not include vacuous defvars such as (defvar foobar), which are generally
used only to quiet the byte-compiler?

In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2011-04-25 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.5) --no-opt --cflags
-Ic:/imagesupport/include'
 






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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-08 18:15 bug#8638: 24.0.50; Imenu should not include vacuous defvars Drew Adams
@ 2011-05-08 18:50 ` Juanma Barranquero
  2011-05-08 19:07   ` Drew Adams
  2011-05-09 14:19   ` Stefan Monnier
  0 siblings, 2 replies; 19+ messages in thread
From: Juanma Barranquero @ 2011-05-08 18:50 UTC (permalink / raw)
  To: Drew Adams; +Cc: 8638

On Sun, May 8, 2011 at 20:15, Drew Adams <drew.adams@oracle.com> wrote:

> Could we please improve `lisp-imenu-generic-expression so that it does
> not include vacuous defvars such as (defvar foobar), which are generally
> used only to quiet the byte-compiler?

With lexical binding, (defvar foobar) is used to tell the bytecompiler
that the variable has dynamic scope.

    Juanma





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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-08 18:50 ` Juanma Barranquero
@ 2011-05-08 19:07   ` Drew Adams
  2011-05-08 19:25     ` Juanma Barranquero
  2011-05-27 16:00     ` Drew Adams
  2011-05-09 14:19   ` Stefan Monnier
  1 sibling, 2 replies; 19+ messages in thread
From: Drew Adams @ 2011-05-08 19:07 UTC (permalink / raw)
  To: 'Juanma Barranquero'; +Cc: 8638

> With lexical binding, (defvar foobar) is used to tell the bytecompiler
> that the variable has dynamic scope.

It's still a vacuous definition.  And any defvar tells the byte compiler that a
variable has dynamic scope, no?

This seems irrelevant to the bug report.  I'd still suggest removing vacuous
defvars from the menu.  Mixing in vacuous entries with entries that really
define variables distracts users.  You want to think that accessing a menu item
will take you to a real variable definition.

If someone wants to provide vacuous defvars in a different submenu from
`Variables' (e.g. `Vacuous Vars') I have no problem with that.  But I don't
really think that's needed.

FWIW, this is what I use in my code (imenu+.el):

(concat "^\\s-*("
        (regexp-opt
          '("defvar" "defconst" "defconstant" "defcustom"
            "defparameter" "define-symbol-macro") t)
        "\\s-+\\(\\sw\\(\\sw\\|\\s_\\)+\\)"
        "\\s-+[^) \t\n]")

Not perfect, perhaps, but it seems to do the job OK so far.






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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-08 19:07   ` Drew Adams
@ 2011-05-08 19:25     ` Juanma Barranquero
  2011-05-08 19:36       ` Drew Adams
  2011-05-27 16:00     ` Drew Adams
  1 sibling, 1 reply; 19+ messages in thread
From: Juanma Barranquero @ 2011-05-08 19:25 UTC (permalink / raw)
  To: Drew Adams; +Cc: 8638

> It's still a vacuous definition.  And any defvar tells the byte compiler that a
> variable has dynamic scope, no?

It also tells the user, so having them in the imenu doesn't seem
superfluous to me (I'm talking again of lexically-scoped packages).

    Juanma





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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-08 19:25     ` Juanma Barranquero
@ 2011-05-08 19:36       ` Drew Adams
  2011-05-08 19:46         ` Juanma Barranquero
  0 siblings, 1 reply; 19+ messages in thread
From: Drew Adams @ 2011-05-08 19:36 UTC (permalink / raw)
  To: 'Juanma Barranquero'; +Cc: 8638

> > It's still a vacuous definition.  And any defvar tells the 
> > byte compiler that a variable has dynamic scope, no?
> 
> It also tells the user, so having them in the imenu doesn't seem
> superfluous to me (I'm talking again of lexically-scoped packages).

Submenu `Variables' should be for variable definitions, not vacuous defvars that
might be used to indicate something to the byte compiler.

As I said, if you want to also present those to the user, then let's put them in
a  separate submenu.  They amount to byte-compiler declarations.  Perhaps there
are other byte-compiler-related constructs that could also be added to the same
submenu.  Call it `Byte-Compiler' or `Declarations', perhaps.

But these are not variable definitions in the same sense as full defvars are.
Mixing them in with full definitions, in the same submenu, just amounts to
noise.  Separating them out in a separate menu would be fine.






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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-08 19:36       ` Drew Adams
@ 2011-05-08 19:46         ` Juanma Barranquero
  2011-05-08 19:46           ` Juanma Barranquero
  2011-05-08 20:03           ` Drew Adams
  0 siblings, 2 replies; 19+ messages in thread
From: Juanma Barranquero @ 2011-05-08 19:46 UTC (permalink / raw)
  To: Drew Adams; +Cc: 8638

On Sun, May 8, 2011 at 21:36, Drew Adams <drew.adams@oracle.com> wrote:

> But these are not variable definitions in the same sense as full defvars are.

I disagree. IMHO, in a lexical binding package, yes, there are
variable definitions. In some cases the variables are documented in
the docstring of a function or somesuch, but they are real variables
nonetheless. Instead of sweeping them under the carpet, perhaps it
would be better to suggest the programmer to add proper docstrings and
initial values to them.

    Juanma





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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-08 19:46         ` Juanma Barranquero
@ 2011-05-08 19:46           ` Juanma Barranquero
  2011-05-08 20:03           ` Drew Adams
  1 sibling, 0 replies; 19+ messages in thread
From: Juanma Barranquero @ 2011-05-08 19:46 UTC (permalink / raw)
  To: Drew Adams; +Cc: 8638

> I disagree. IMHO, in a lexical binding package, yes, there are

s/there/they/

    Juanma





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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-08 19:46         ` Juanma Barranquero
  2011-05-08 19:46           ` Juanma Barranquero
@ 2011-05-08 20:03           ` Drew Adams
  2011-05-08 20:29             ` Juanma Barranquero
  1 sibling, 1 reply; 19+ messages in thread
From: Drew Adams @ 2011-05-08 20:03 UTC (permalink / raw)
  To: 'Juanma Barranquero'; +Cc: 8638

> I disagree. IMHO, in a lexical binding package, yes, there are
> variable definitions. In some cases the variables are documented in
> the docstring of a function or somesuch, but they are real variables
> nonetheless. Instead of sweeping them under the carpet, perhaps it
> would be better to suggest the programmer to add proper docstrings and
> initial values to them.

No one is sweeping anything under the carpet.
If you want to show them in an Imenu menu, fine; just don't mix them in with
definitions that people will want to visit to see doc strings and initial
values.

That programmers should be encouraged to use doc strings and specify initial
values is a separate issue.  That does not imply that vacuous defvars should be
included in the Variables menu.

As a signal to the byte compiler, a vacuous definition is useful - as such.
That's what it is for.  By definition it should not have an initial value or doc
string.

That a vacuous definition, like a full one, is now used also to declare a
variable special (dynamic scoping) does not mean that vacuous defvars should be
included in the Variables menu.

Whether a vacuous definition should indicate dynamic scope to the byte-compiler
is another question.  As you say, in most cases what we want to suggest is that
programmers use a full definition for that, instead.  But using a vacuous
definition to quiet undefined var warnings is legitimate - in that case the
programmer does _not_ want to include any initial value.

IMO, you are mixing in things that don't belong to this thread.  A defvar that
is used _only_ as a byte-compiler declaration and not to provide an initial
value (and hopefully a doc string) does not belong in the same submenu as full
definitions.

When you follow a Variables menu entry to its code, you want to see what the
code for the variable is.  You do not want to see only a vacuous defvar that
provides no more information than the menu item itself.






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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-08 20:03           ` Drew Adams
@ 2011-05-08 20:29             ` Juanma Barranquero
  2011-05-08 20:39               ` Drew Adams
  0 siblings, 1 reply; 19+ messages in thread
From: Juanma Barranquero @ 2011-05-08 20:29 UTC (permalink / raw)
  To: Drew Adams; +Cc: 8638

On Sun, May 8, 2011 at 22:03, Drew Adams <drew.adams@oracle.com> wrote:

> When you follow a Variables menu entry to its code, you want to see what the
> code for the variable is.  You do not want to see only a vacuous defvar that
> provides no more information than the menu item itself.

The imenu index is not documentation. It is an index to code. So if
the "vacuous variable" has twenty lines of comment explaining its
purpose and why it does not have or require a docstring or an initial
value, I fully expect imenu to help me get there too, just as if it
were a normal, "full" variable. Your expectations for imenu are just
that, your expectations. I prefer to be the judge of what it is
interesting in the code and what is not, not some imenu filter.

But that's just my opinion. I'm not going to complain if someone fixes
this bug to your liking.

    Juanma





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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-08 20:29             ` Juanma Barranquero
@ 2011-05-08 20:39               ` Drew Adams
  2011-05-08 20:52                 ` Juanma Barranquero
  0 siblings, 1 reply; 19+ messages in thread
From: Drew Adams @ 2011-05-08 20:39 UTC (permalink / raw)
  To: 'Juanma Barranquero'; +Cc: 8638

> > When you follow a Variables menu entry to its code, you 
> > want to see what the code for the variable is.  You do not
> > want to see only a vacuous defvar that
> > provides no more information than the menu item itself.
> 
> The imenu index is not documentation. It is an index to code. So if
> the "vacuous variable" has twenty lines of comment explaining its
> purpose and why it does not have or require a docstring or an initial
> value, I fully expect imenu to help me get there too, just as if it
> were a normal, "full" variable. Your expectations for imenu are just
> that, your expectations. I prefer to be the judge of what it is
> interesting in the code and what is not, not some imenu filter.
> 
> But that's just my opinion. I'm not going to complain if someone fixes
> this bug to your liking.

I understand.  How would you feel if Imenu included entries for defuns and
defvars that are commented out?  Wouldn't you want Imenu to judge that you are
not interested in those?

Certainly comments that include defuns and such could be important and
interesting to developers.  The question is what Imenu should include, and yes,
it is a judgment call based on expectations of what most users will want/expect.






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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-08 20:39               ` Drew Adams
@ 2011-05-08 20:52                 ` Juanma Barranquero
  2011-05-08 21:49                   ` Drew Adams
  0 siblings, 1 reply; 19+ messages in thread
From: Juanma Barranquero @ 2011-05-08 20:52 UTC (permalink / raw)
  To: Drew Adams; +Cc: 8638

On Sun, May 8, 2011 at 22:39, Drew Adams <drew.adams@oracle.com> wrote:

> I understand.  How would you feel if Imenu included entries for defuns and
> defvars that are commented out?  Wouldn't you want Imenu to judge that you are
> not interested in those?

Well, we're not talking about commented out variables, so the
comparison isn't entirely fair.

But, to be honest, sometimes I think that entries for commented out
defuns and defvars would be useful... :-)

> Certainly comments that include defuns and such could be important and
> interesting to developers.  The question is what Imenu should include, and yes,
> it is a judgment call based on expectations of what most users will want/expect.

Not users. Developers. And developers are usually interested in a
package's variables. More so in a lexical-binding context, where the
fact that the defvar exists can (and does) alter semantics.

    Juanma





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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-08 20:52                 ` Juanma Barranquero
@ 2011-05-08 21:49                   ` Drew Adams
  0 siblings, 0 replies; 19+ messages in thread
From: Drew Adams @ 2011-05-08 21:49 UTC (permalink / raw)
  To: 'Juanma Barranquero'; +Cc: 8638

> > it is a judgment call based on expectations of what most 
> > users will want/expect.
> 
> Not users. Developers. And developers are usually interested in a
> package's variables. More so in a lexical-binding context, where the
> fact that the defvar exists can (and does) alter semantics.

Users includes users who develop, and more.  Imenu is for all Emacs users,
including developers.  And the question is not whether developers are interested
in a package's variables.  It is whether the Variables submenu should include
entries for vacuous defvars.






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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-08 18:50 ` Juanma Barranquero
  2011-05-08 19:07   ` Drew Adams
@ 2011-05-09 14:19   ` Stefan Monnier
  2011-05-09 14:31     ` Juanma Barranquero
  1 sibling, 1 reply; 19+ messages in thread
From: Stefan Monnier @ 2011-05-09 14:19 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: 8638

>> Could we please improve `lisp-imenu-generic-expression so that it does
>> not include vacuous defvars such as (defvar foobar), which are generally
>> used only to quiet the byte-compiler?
> With lexical binding, (defvar foobar) is used to tell the bytecompiler
> that the variable has dynamic scope.

While it is true some some (defvar <foo>) are actually declarations that
<foo> is a locally-used dynamically bound variable (in which case,
maybe it could make sense to see it in imenu), the overwhelming
majority is to declare the existence of some variable in some other
package, in which case I don't think it deserves to be in imenu.
And even when it might make sense, its usefulness seems dubious since
those defvars don't hold much valuable info (like initial value,
docstring, or something).
So I'd tend to agree with Drew.  For completion purposes, we'd want to
pay attention to those defvars, but not for imenu.


        Stefan





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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-09 14:19   ` Stefan Monnier
@ 2011-05-09 14:31     ` Juanma Barranquero
  0 siblings, 0 replies; 19+ messages in thread
From: Juanma Barranquero @ 2011-05-09 14:31 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 8638

On Mon, May 9, 2011 at 16:19, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> So I'd tend to agree with Drew.  For completion purposes, we'd want to
> pay attention to those defvars, but not for imenu.

I had already conceded (Warnock style, interpretation #1 ;-)

    Juanma





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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-08 19:07   ` Drew Adams
  2011-05-08 19:25     ` Juanma Barranquero
@ 2011-05-27 16:00     ` Drew Adams
  2012-08-05 14:15       ` Chong Yidong
  1 sibling, 1 reply; 19+ messages in thread
From: Drew Adams @ 2011-05-27 16:00 UTC (permalink / raw)
  To: 8638

> FWIW, this is what I use in my code (imenu+.el):
> (concat "^\\s-*("
>         (regexp-opt
>           '("defvar" "defconst" "defconstant" "defcustom"
>             "defparameter" "define-symbol-macro") t)
>         "\\s-+\\(\\sw\\(\\sw\\|\\s_\\)+\\)"
>         "\\s-+[^) \t\n]")
> Not perfect, perhaps, but it seems to do the job OK so far.


FWIW, I use this now.
Changed the whitespace match after var name.

(concat "^\\s-*("
        (regexp-opt
          '("defvar" "defconst" "defconstant" "defcustom"
            "defparameter" "define-symbol-macro") t)
        "\\s-+\\(\\sw\\(\\sw\\|\\s_\\)+\\)"
        "\\(\\s-\\|[\n]\\)+" ; \n has char syntax `>', not `-'
        "[^) \t\n]")

\s-+ does not match newlines in Lisp modes (newlines have comment-end syntax),
so I changed \s-+ to \(\s-\|[\n]\)+.  No doubt still not perfect, but seems to
work OK.






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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2011-05-27 16:00     ` Drew Adams
@ 2012-08-05 14:15       ` Chong Yidong
  2012-08-05 16:27         ` Drew Adams
  0 siblings, 1 reply; 19+ messages in thread
From: Chong Yidong @ 2012-08-05 14:15 UTC (permalink / raw)
  To: Drew Adams; +Cc: 8638

"Drew Adams" <drew.adams@oracle.com> writes:

> FWIW, I use this now.
> Changed the whitespace match after var name.

I committed a slightly different fix to trunk.  Thanks.





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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2012-08-05 14:15       ` Chong Yidong
@ 2012-08-05 16:27         ` Drew Adams
  2012-08-06  3:43           ` Chong Yidong
  0 siblings, 1 reply; 19+ messages in thread
From: Drew Adams @ 2012-08-05 16:27 UTC (permalink / raw)
  To: 'Chong Yidong'; +Cc: 8638

> > FWIW, I use this now.
> > Changed the whitespace match after var name.
> 
> I committed a slightly different fix to trunk.  Thanks.

OK, thanks.  But I wonder why you treated defvar differently from defconst,
defconstant, defcustom, defparameter, and define-symbol-macro here.  Shouldn't
the same thing apply to them?

Just wondering.  Not that their use of a vacuous definition would mean the same
thing, or even necessarily by correct syntax.  But shouldn't a vacuous
definition using one of those others also be ignored by Imenu?






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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2012-08-05 16:27         ` Drew Adams
@ 2012-08-06  3:43           ` Chong Yidong
  2012-08-06  3:52             ` Drew Adams
  0 siblings, 1 reply; 19+ messages in thread
From: Chong Yidong @ 2012-08-06  3:43 UTC (permalink / raw)
  To: Drew Adams; +Cc: 8638

"Drew Adams" <drew.adams@oracle.com> writes:

> OK, thanks.  But I wonder why you treated defvar differently from
> defconst, defconstant, defcustom, defparameter, and
> define-symbol-macro here.  Shouldn't the same thing apply to them?

There is no such thing as (defconst foo), or (defcustom foo); the second
argument is non-optional.  So the reasoning which was used for defvar,
i.e. that a defvar with an omitted second arg is commonly used just to
silence the compiler, does not apply.





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

* bug#8638: 24.0.50; Imenu should not include vacuous defvars
  2012-08-06  3:43           ` Chong Yidong
@ 2012-08-06  3:52             ` Drew Adams
  0 siblings, 0 replies; 19+ messages in thread
From: Drew Adams @ 2012-08-06  3:52 UTC (permalink / raw)
  To: 'Chong Yidong'; +Cc: 8638

> > OK, thanks.  But I wonder why you treated defvar differently from
> > defconst, defconstant, defcustom, defparameter, and
> > define-symbol-macro here.  Shouldn't the same thing apply to them?
> 
> There is no such thing as (defconst foo), or (defcustom foo); 
> the second argument is non-optional.

Yes, I know.

> So the reasoning which was used for defvar, i.e. that a defvar
> with an omitted second arg is commonly used just to silence the
> compiler, does not apply.

But the reasoning that such things, if they ever occurred, would not represent
proper definitions, so Imenu should not index them, holds.  They would represent
incorrect syntax (i.e., errors), and should not be indexed.

Anyway, I'm OK with them being falsely indexed, and I would agree if you made
the argument that we do not try to prevent indexing of incorrect syntax in
general.
 






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

end of thread, other threads:[~2012-08-06  3:52 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-08 18:15 bug#8638: 24.0.50; Imenu should not include vacuous defvars Drew Adams
2011-05-08 18:50 ` Juanma Barranquero
2011-05-08 19:07   ` Drew Adams
2011-05-08 19:25     ` Juanma Barranquero
2011-05-08 19:36       ` Drew Adams
2011-05-08 19:46         ` Juanma Barranquero
2011-05-08 19:46           ` Juanma Barranquero
2011-05-08 20:03           ` Drew Adams
2011-05-08 20:29             ` Juanma Barranquero
2011-05-08 20:39               ` Drew Adams
2011-05-08 20:52                 ` Juanma Barranquero
2011-05-08 21:49                   ` Drew Adams
2011-05-27 16:00     ` Drew Adams
2012-08-05 14:15       ` Chong Yidong
2012-08-05 16:27         ` Drew Adams
2012-08-06  3:43           ` Chong Yidong
2012-08-06  3:52             ` Drew Adams
2011-05-09 14:19   ` Stefan Monnier
2011-05-09 14:31     ` Juanma Barranquero

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