show-modifiers-before-signatures patch for ScalaDoc

At first glance it looks good. And I like the idea. :-)

Btw. regarding Scaladoc, I noticed that recently the signatures have a
drop shadow again. I don't really like it. It's fancy but it also
makes the signature hard to read, so I would prefer a plain font
without the drop shadow.

Regards,
Rüdiger

2011/5/8 Simon Ochsenreither :
> Hi,
>
> I wrote a patch to show modifiers like "final", "sealed", "abstract",
> "implicit" before the signature in ScalaDoc (i. e. how people would expect
> it from reading source code).
>
> Example how it looks like (with an older version of ScalaDoc) here:
> http://oxnrtr.de/ScalaDoc/#scala.collection.immutable.List )
>
> The change would require some additional amount of space before the method
> signatures to accommodate the modifiers, that's why I'm asking here.
>
> Ticket is #4451 http://lampsvn.epfl.ch/trac/scala/ticket/4451
> Patch is
> https://github.com/soc/scala/commit/ffa098c549ec2bc8b6e72ad71c153061b9da...
>
> What do you think?
>
>
> Thanks and bye,
>
>
> Simon
>

Very useful.

Perhaps the spacing would look better if modifiers (when present) got a line of their own above the main "def" line.

A

On Sun, May 8, 2011 at 10:39 AM, Simon Ochsenreither <simon@ochsenreither.de> wrote:

Hi,

I wrote a patch to show modifiers like "final", "sealed", "abstract", "implicit" before the signature in ScalaDoc (i. e. how people would expect it from reading source code).

Example how it looks like (with an older version of ScalaDoc) here: http://oxnrtr.de/ScalaDoc/#scala.collection.immutable.List )

The change would require some additional amount of space before the method signatures to accommodate the modifiers, that's why I'm asking here.

Ticket is #4451 http://lampsvn.epfl.ch/trac/scala/ticket/4451
Patch is https://github.com/soc/scala/commit/ffa098c549ec2bc8b6e72ad71c153061b9daf4f1

What do you think?


Thanks and bye,


Simon

#avg_ls_inline_popup{position: absolute;z-index: 9999;padding: 0px 0px;margin-left: 0px;margin-top: 0px;overflow: hidden;word-wrap: break-word;color: #000;font-size: 10px;text-align: left;line-height: 130%;}

On Sunday May 8 2011, Archontophoenix Quar wrote:
> Very useful.
>
> Perhaps the spacing would look better if modifiers (when present) got
> a line of their own above the main "def" line.

Hallelujah!

This is how I write Scala, actually. (Need I say I wish everyone did?
No, of course I need not...)

> A

Randall Schulz

Hi,
> Perhaps the spacing would look better if modifiers (when present) got
> a line of their own above the main "def" line.

I thought about that too. But imho it creates more visual clutter than
the other option (and it looks like some butt-ugly C++ style (imho)). I
also thought about making the doc start under "def"/"class" instead of
after it, but that doesn't look too good.

Bye,

SImon

There's something wrong. It shows a bunch of members as "abstract"
when they clearly are not.

All abstract members are segregated in a separate section in the
beginning, though not type members. Implicit is already shown right
now, but final and sealed are not.

Honestly, I'm not sure. I do like having this information -- I tested
it by looking at Parsers, and it revealed some stuff I wasn't aware of

Hi Daniel,

> There's something wrong. It shows a bunch of members as "abstract"
> when they clearly are not.

This is possible.

> All abstract members are segregated in a separate section in the
> beginning, though not type members. Implicit is already shown right
> now, but final and sealed are not.

Could you link to some examples? I'm having problems following you here.

> Honestly, I'm not sure. I do like having this information -- I tested
> it by looking at Parsers, and it revealed some stuff I wasn't aware of

On Mon, May 9, 2011 at 09:51, Simon Ochsenreither
wrote:
> Hi Daniel,
>
>> There's something wrong. It shows a bunch of members as "abstract"
>> when they clearly are not.
>
> This is possible.
>
>
>> All abstract members are segregated in a separate section in the
>> beginning, though not type members. Implicit is already shown right
>> now, but final and sealed are not.
>
> Could you link to some examples? I'm having problems following you here.

See, below, the section "Abstract Value Members". All abstract defs,
vars and vals appear in that section, everything outside it is not
abstract. This is particularly more valuable than just flagging
something abstract because it concentrates every method you'll have to
implement in one place.

http://www.scala-lang.org/archives/downloads/distrib/files/nightly/docs/...

Now see below that Elem is not, likewise, segregated. It is abstract,
which is indicated by clicking on it to show its attributes.

http://www.scala-lang.org/archives/downloads/distrib/files/nightly/docs/...

Other than that, List -- Class and Object -- and Parsers show clearly
that "final" and "sealed" are not presently shown, but "implicit" is.

So your patch adds three things: abstract, final and sealed. Of these
three things, abstract's handling is unnecessary for value members,
but necessary for type members.

That's all I was saying.

>> Honestly, I'm not sure. I do like having this information -- I tested
>> it by looking at Parsers, and it revealed some stuff I wasn't aware of

I also think `abstract' should not be shown at all as an attribute. For value members, it's implied by the section they are in. For type members it's implied by the definition (alias type or bounds). So it fulfills no purpose.

And there are several reasons why it is confusing: First, abstract is not a member modifier in Scala. Second, abstract for types is quite different to abstract for values. For instance, a class containing abstract type members may very well be instantiated where a class containing abstract value members may not.

Cheers
 
 -- Martin

On Mon, May 9, 2011 at 4:46 PM, Daniel Sobral <dcsobral@gmail.com> wrote:

On Mon, May 9, 2011 at 09:51, Simon Ochsenreither
<simon@ochsenreither.de> wrote:
> Hi Daniel,
>
>> There's something wrong. It shows a bunch of members as "abstract"
>> when they clearly are not.
>
> This is possible.
>
>
>> All abstract members are segregated in a separate section in the
>> beginning, though not type members. Implicit is already shown right
>> now, but final and sealed are not.
>
> Could you link to some examples? I'm having problems following you here.

See, below, the section "Abstract Value Members". All abstract defs,
vars and vals appear in that section, everything outside it is not
abstract. This is particularly more valuable than just flagging
something abstract because it concentrates every method you'll have to
implement in one place.

http://www.scala-lang.org/archives/downloads/distrib/files/nightly/docs/library/index.html#scala.collection.immutable.List

Now see below that Elem is not, likewise, segregated. It is abstract,
which is indicated by clicking on it to show its attributes.

http://www.scala-lang.org/archives/downloads/distrib/files/nightly/docs/library/index.html#scala.util.parsing.combinator.Parsers

Other than that, List -- Class and Object -- and Parsers show clearly
that "final" and "sealed" are not presently shown, but "implicit" is.

So your patch adds three things: abstract, final and sealed. Of these
three things, abstract's handling is unnecessary for value members,
but necessary for type members.

That's all I was saying.

>> Honestly, I'm not sure. I do like having this information -- I tested
>> it by looking at Parsers, and it revealed some stuff I wasn't aware of
>> -- but some type signatures get really long. I'd prefer to have short,
>> colored icons: A, S, F, I. It gives the same amount of information in
>> a smaller footprint.
>
> Yes, the necessary space is my biggest concern, too. But imho replacing the
> modifiers with icons in these text-heavy places doesn't felt right when I
> tried it. Additionally, having the same representation in source/ScalaDoc is
> a benefit, because people don't have to learn icons.
>
>
>> And, anyway, "abstract" need to be fixed.
>
> Could you show me an example? Most likely it was wrong the whole times
> (because I didn't change anything related to that) and just no one saw it. I
> just changed the way it is displayed, not the way how these modifiers are
> figured out.

Actually, I figured out what the problems seems to be. Every use case
is being tagged "abstract". At any rate, anything on Concrete Value
Members that is tagged "abstract" just isn't.


>
> But I can have a look at it.
>
>



--
Daniel C. Sobral

I travel to the future all the time.

--
----------------------------------------------
Martin Odersky
Prof., EPFL and CEO, Scala Solutions
PSED, 1015 Lausanne, Switzerland

On Mon, May 9, 2011 at 09:51, Simon Ochsenreither
wrote:
>
> Could you show me an example? Most likely it was wrong the whole times
> (because I didn't change anything related to that) and just no one saw it. I
> just changed the way it is displayed, not the way how these modifiers are
> figured out.

And it is true that it is wrong currently as well! I just didn't
notice it because, even though it has an "attributes: abstract", it
shows correctly on Concrete Value Members.

On Mon, May 9, 2011 at 11:55, martin odersky wrote:
> I also think `abstract' should not be shown at all as an attribute. For
> value members, it's implied by the section they are in. For type members
> it's implied by the definition (alias type or bounds). So it fulfills no
> purpose.

Not quite true. Taking Parsers as an example, NoSuccess is abstract,
so it can't be instantiated. It is also sealed, so no one would be
subclassing it, but that may not have been true.

Type alias or bounds, indeed, are more obviously abstract.

> And there are several reasons why it is confusing: First, abstract is not a
> member modifier in Scala. Second, abstract for types is quite different to
> abstract for values. For instance, a class containing abstract type members
> may very well be instantiated where a class containing abstract value
> members may not.

Learn something new every day. I didn't realize one could instantiate
classes containing abstract type members.

On Mon, May 9, 2011 at 5:18 PM, Daniel Sobral wrote:
> On Mon, May 9, 2011 at 11:55, martin odersky wrote:
>> And there are several reasons why it is confusing: First, abstract is not a
>> member modifier in Scala. Second, abstract for types is quite different to
>> abstract for values. For instance, a class containing abstract type members
>> may very well be instantiated where a class containing abstract value
>> members may not.
>
> Learn something new every day. I didn't realize one could instantiate
> classes containing abstract type members.

Although not in objects.

scala> object A { type A }
:5: error: only classes can have declared but undefined members
object A { type A }
^

-jason

On Mon, May 9, 2011 at 5:18 PM, Daniel Sobral <dcsobral@gmail.com> wrote:

On Mon, May 9, 2011 at 11:55, martin odersky <martin.odersky@epfl.ch> wrote:
> I also think `abstract' should not be shown at all as an attribute. For
> value members, it's implied by the section they are in. For type members
> it's implied by the definition (alias type or bounds). So it fulfills no
> purpose.

Not quite true. Taking Parsers as an example, NoSuccess is abstract,
so it can't be instantiated. It is also sealed, so no one would be
subclassing it, but that may not have been true.

That's actually a very good point. Abstract for classes means something completely different from abstract for other members. Scala has an `abstract' modifier only for classes.

An abstract class is a class that cannot be instantiated.

An abstract value member is a member without implementation; it needs to be implemented before the enclosing class or trait can be instantiated.

 An abstract type member is given by its bounds rather than a concrete type alias.

In the compiler, abstract classes are marked with the ABSTRACT flag. All other forms of `abstract' are marked with the DEFERRED flag.

Classes with an ABSTRACT flag should have their attribute shown in scaladoc. The rest should be ignored.

Cheers

 -- Martin

Thanks everyone for your thoughts!

I agree that some abstract modifiers aren't necessary to be shown at all
and disabling it were it doesn't make sense would be the next step.

My original patch was a more cosmetic one and more targeted at placing
the modifiers where most people expect them instead of having them in
some attribute table somewhere else.

Thanks and bye,

Simon

Hi Simon,

will you continue working on this?

Regards,
Rüdiger

2011/5/9 Simon Ochsenreither :
> Thanks everyone for your thoughts!
>
> I agree that some abstract modifiers aren't necessary to be shown at all and
> disabling it were it doesn't make sense would be the next step.
>
> My original patch was a more cosmetic one and more targeted at placing the
> modifiers where most people expect them instead of having them in some
> attribute table somewhere else.
>
> Thanks and bye,
>
>
> Simon
>

Hi Ruediger,

I'm quite busy at the moment and I fear that I'm not good enough to
incorporate all these additional rules about where to show it and where
not, so fell free to pick it up if you want.

I guess someone who knows the inner working of ScalaDoc a bit better
might be able to fix the remaining issues much faster than me, anyway.

On the other hand, the current patch could be committed as is, it
doesn't cause any regressions, but has shown some places where the code
was worng all the time already.

>> Thanks everyone for your thoughts!
>>
>> I agree that some abstract modifiers aren't necessary to be shown at all and
>> disabling it were it doesn't make sense would be the next step.
>>
>> My original patch was a more cosmetic one and more targeted at placing the
>> modifiers where most people expect them instead of having them in some
>> attribute table somewhere else.
>>

Thanks and bye,

Simon

Hi Simon,

it's a pity you don't have any more time to work on this. I think it's
a nice improvement.

I don't know yet if/when I will be able to do this. I wanted to do
some other things for Scaladoc first.

But I guess it won't be forgotten, IIRC you created a ticket for it.

Regards,
Rüdiger

2011/5/20 Simon Ochsenreither :
> Hi Ruediger,
>
> I'm quite busy at the moment and I fear that I'm not good enough to
> incorporate all these additional rules about where to show it and where not,
> so fell free to pick it up if you want.
>
> I guess someone who knows the inner working of ScalaDoc a bit better might
> be able to fix the remaining issues much faster than me, anyway.
>
> On the other hand, the current patch could be committed as is, it doesn't
> cause any regressions, but has shown some places where the code was worng
> all the time already.
>
>>> Thanks everyone for your thoughts!
>>>
>>> I agree that some abstract modifiers aren't necessary to be shown at all
>>> and
>>> disabling it were it doesn't make sense would be the next step.
>>>
>>> My original patch was a more cosmetic one and more targeted at placing
>>> the
>>> modifiers where most people expect them instead of having them in some
>>> attribute table somewhere else.
>>>
>
> Thanks and bye,
>
>
> Simon
>

Hi Rüdiger,

I cleaned it up and did a pull request:
https://github.com/scala/scala/pull/26

The ticket number is https://issues.scala-lang.org/browse/SI-4451 .

> it's a pity you don't have any more time to work on this. I think it's
> a nice improvement.
>
> I don't know yet if/when I will be able to do this. I wanted to do
> some other things for Scaladoc first.
>
> But I guess it won't be forgotten, IIRC you created a ticket for it.

An additional patch which builds on top of it is
https://github.com/oberon-team/scala/commits/experimental/anyval-value
(which concentrates on reducing that "extends Any with AnyVal"/"extends
AnyRef" bloat in ScalaDoc by introducing a "special" kind for value
types, currently named "value") it was discussed before on the
scala-debate list, but without further progress, afair.

Bye,

Simon

That's great. I'm still working on the member filter rewrite and I
think that will take a few days, because I don't have too much time on
my hands.

Regards,
Rüdiger

2011/5/22 Simon Ochsenreither :
> Hi Rüdiger,
>
> I cleaned it up and did a pull request:
> https://github.com/scala/scala/pull/26
>
> The ticket number is https://issues.scala-lang.org/browse/SI-4451 .
>
>> it's a pity you don't have any more time to work on this. I think it's
>> a nice improvement.
>>
>> I don't know yet if/when I will be able to do this. I wanted to do
>> some other things for Scaladoc first.
>>
>> But I guess it won't be forgotten, IIRC you created a ticket for it.
>
> An additional patch which builds on top of it is
> https://github.com/oberon-team/scala/commits/experimental/anyval-value
> (which concentrates on reducing that "extends Any with AnyVal"/"extends
> AnyRef" bloat in ScalaDoc by introducing a "special" kind for value types,
> currently named "value") it was discussed before on the scala-debate list,
> but without further progress, afair.
>
> Bye,
>
>
> Simon
>