Closed GoogleCodeExporter closed 8 years ago
Forgot test case.
function ClassObject() {
/**
* @public
* @type Object
*/
var prop = null;
this.__defineGetter__("prop", function() { return (prop); });
this.__defineSetter__("prop", function(value) { return (prop = value); });
}
Original comment by mcbain....@gmail.com
on 25 Jun 2008 at 4:33
Apparently -p controls private, _vars, and inner (tagged @public?).
That's less than ideal for authors like me who tag everything (all inner
variables,
etc) and just marked those not wanted seen outside as @private. This means to
get the
inner @public variables I want, I have to enable -p. This will then output all
of
those @private variables I don't want the end users of my code to see (the
comments
were meant for the library programmers/maintainers). This is most annoying.
Is there any way that this could be split into a separate command-line
switch/option
or at least have a different behavior?
Original comment by mcbain....@gmail.com
on 25 Jun 2008 at 5:03
you are correct: -p should affect variables that are underscored, marked
@private or
inner. All three of those situations should be treated as a "private variable"
--
JavaScript doesn't (yet) have actual private variables so I feel free to invent
my
own for documentation purposes.
However the correct bahavior of @public should be to remove the 'private-ness'
of any
variable it is applied to (as if they were not private). The -p option by
contrast
will cause all private variables appear in the documentation, but they will
still be
marked as "private".
If you really want a variable to be "marked as not wanted seen outside" use the
@ignore tag. Ignored variables never appear in the documentation.
The bug here is that and underscored variable is still considered private,
despite
the @public tag?
Original comment by micmath
on 25 Jun 2008 at 10:15
Oh, I knew there was @ignore. I still want the ability to be able to run the
toolkit
with private on, in case it becomes helpful to developers.
As for the bug, *any* inner variable marked with @public is trapped by -p, not
just
those with underscores.
Original comment by mcbain....@gmail.com
on 25 Jun 2008 at 5:49
I'm having trouble understanding exactly what you are asking for, so just
confirm
this is a valid testcase for the bug, shown below.
- The expected behavior (with no -p option present) is for ClassObject#foo and
ClassObject#_foz to appear in the documentation.
- The current behavior is that they don't appear in the documentation.
/** @constructor */
function ClassObject() {
/**
* @public
*/
var foo = {};
/**
* @public
*/
this._foz = {};
}
Secondly there is a feature request: that you would like to be able to select
*either* inner, underscored, or @private symbols to be documented, based on a
command
line switch?
Original comment by micmath
on 26 Jun 2008 at 9:33
Yes, that is a valid test case. Since they are marked @public, they should
appear in
the documentation. They currently are not passed to the template (verified while
writing my template).
As for the "feature request", this is what I was originally asking: I was
originally
unsure if this was a real bug, or an intended feature. If it was a feature, I
was
asking if it could be overridden with a command line switch, so that @public
variables could be documented independently of -p. Since this is actually a
bug, the
question is no longer valid.
I hope this helps.
Original comment by mcbain....@gmail.com
on 26 Jun 2008 at 4:03
Yay! I think I found the location of the bug.
Line 41 of parser.js looks like this right now:
if ((symbol.isInner || symbol.isPrivate) && !JSDOC.opt.p) return;
It should/could be:
if (!JSDOC.opt.p && ((symbol.isInner && symbol.isPrivate) || (!symbol.isInner &&
symbol.isPrivate))) return;
Personally, my response is "Ow ow ow, my head hurts." :P for the following
reasons:
1) On the first 5,000 or so surface looks, your code would appear to be exactly
what
is wanted/needed. However since it is an "or", if the first condition is false
it
moves on to the next one, which in our test case would evaluate true.
2) My code (which should fix the bug)looks very ripe for refactoring to make it
shorter and more concise ... but then you realize that you'd end up turning it
into
something similar to what you just fixed.
3) I made a seriously boneheaded mistake in my own project last night and ended
up
going through at least 3 full class files about 20 times before I realized what
the
error was. My brain is still sore. :P
I hope this helps, and I did a silly "micro-optimization" in that I moved the
check
for the JsDoc command line option first, so that if it fails we don't bother
checking
properties. It'll probably save < 1 millisecond of parsing time. :P
P.S. I don't have any code to test this on right now, but I can later to night.
Good
luck.
Original comment by mcbain....@gmail.com
on 26 Jun 2008 at 6:08
And one last comment. From the looks of the original Parse.js line I found
(assuming
it is the only place that kind of sorting is done), properties starting with
underscores would not have been affected by this bug, only inner variables. So
your
proposed test case would show the bug for foo, but not foz.
Original comment by mcbain....@gmail.com
on 26 Jun 2008 at 6:19
well, I tried my patch and it works. However, I think it reveals another
potential
bug/feature. Are inner variables that have no explicit @public or @private tag
public? With my patch, inner variables that are not labeled @public or @private
are
outputted as inner variables with the -p option off.
However, let's not complicate issues, and if this is a separate issue, we
should make
it so after this bug is fixed. If that is intended, it can just be dropped.
Original comment by mcbain....@gmail.com
on 26 Jun 2008 at 9:57
I think the code you've written is logically equivalent to:
if (!JSDOC.opt.p && symbol.isPrivate) return;
The logical (symbol.isInner || !symbol.isInner) cancels itself out and so can
be factored away.
After looking at this I believe there are actually 2 bugs. The first you've
correctly identified: I shouldn't be checking isInner
OR isPrivate because inner variables are explicitly marked as private on line
445 of lib/JSDOC/Symbol.js. I should only be
checking isPrivate.
The second bug is that on line 37 of lib/JSDOC/Parser.js underscored variables
are marked as private regardless of any
@public tag.
So combining everything from this discussion I get this patch:
Index: app/lib/JSDOC/Parser.js
===================================================================
--- app/lib/JSDOC/Parser.js (revision 628)
+++ app/lib/JSDOC/Parser.js (working copy)
@@ -34,11 +34,11 @@
// uderscored things may be treated as if they were marked private, this cascades
if (JSDOC.Parser.conf.treatUnderscoredAsPrivate && symbol.name.match(/[.#-]_[^.#-]+$/)) {
- symbol.isPrivate = true;
+ if (!symbol.comment.getTag("public")) symbol.isPrivate
= true;
}
// -p flag is required to document private things
- if ((symbol.isInner || symbol.isPrivate) && !JSDOC.opt.p)
return;
+ if (!JSDOC.opt.p && symbol.isPrivate) return; // issue #161
fixed by mcbain.asm
// ignored things are not documented, this doesn't cascade
if (symbol.isIgnored) return;
I need to write some unit tests to ensure this bug stays dead, but I will
commit the changes when I am certain that this is
fixed and covered by a test case.
Thanks for your help.
Original comment by micmath
on 28 Jun 2008 at 9:51
Original comment by micmath
on 2 Aug 2008 at 8:18
Original issue reported on code.google.com by
mcbain....@gmail.com
on 25 Jun 2008 at 4:31