[vwnc] In-image documentation bug in VW7.6

A repost but the original was never assigned an AR as far as I know.
This is in VW7.6:

Image>>copy:from:in:rule:

does not document the rule: argument.  I eventually found the
documentation by looking for senders.  For example:

BrowserTabApplication>>eraseBackgroundOnImage: anImage

has an in-method comment describing the bitblt rules.  Several other
methods have the same comment.  This comment would make more sense in
the Image class.

David

_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc

AR 55951...

-----Original Message-----
From: [hidden email] [mailto:[hidden email]] On
Behalf Of C. David Shaffer
Sent: Wednesday, November 19, 2008 6:22 AM
To: [hidden email]
Subject: [vwnc] In-image documentation bug in VW7.6

A repost but the original was never assigned an AR as far as I know.
This is in VW7.6:

Image>>copy:from:in:rule:

does not document the rule: argument.  I eventually found the
documentation by looking for senders.  For example:

BrowserTabApplication>>eraseBackgroundOnImage: anImage

has an in-method comment describing the bitblt rules.  Several other
methods have the same comment.  This comment would make more sense in
the Image class.

David

_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc

_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc

However, the rules are documented in

ByteArray>>copyBitsStride: destStride width: destWidth atX: destX y:
destY from: sourceByteArrayOrNil stride: sourceStride width: sourceWidth
atX: sourceX y: sourceY width: width height: height rule:
combinationRule

AR rejected.

Andres.

-----Original Message-----
From: [hidden email] [mailto:[hidden email]] On
Behalf Of Valloud, Andres
Sent: Thursday, January 01, 2009 11:06 AM
To: VWNC,
Subject: Re: [vwnc] In-image documentation bug in VW7.6

AR 55951...

-----Original Message-----
From: [hidden email] [mailto:[hidden email]] On
Behalf Of C. David Shaffer
Sent: Wednesday, November 19, 2008 6:22 AM
To: [hidden email]
Subject: [vwnc] In-image documentation bug in VW7.6

A repost but the original was never assigned an AR as far as I know.
This is in VW7.6:

Image>>copy:from:in:rule:

does not document the rule: argument.  I eventually found the
documentation by looking for senders.  For example:

BrowserTabApplication>>eraseBackgroundOnImage: anImage

has an in-method comment describing the bitblt rules.  Several other
methods have the same comment.  This comment would make more sense in
the Image class.

David

_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc

_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc

_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc

Valloud, Andres wrote:
> However, the rules are documented in
>
> ByteArray>>copyBitsStride: destStride width: destWidth atX: destX y:
> destY from: sourceByteArrayOrNil stride: sourceStride width: sourceWidth
> atX: sourceX y: sourceY width: width height: height rule:
> combinationRule
>
> AR rejected.
>  
Ouch :-)  Rejection comes hard to me :-) :-)

FWIW there were two parts to the bug report:

1) Image>>copy:from:in:rule:  -- I'll give you this one since it is only
a three step process (view copy:from:in:rule: implementors,
implementors) to get the the docs on rule.  Some comment hyperlinking
would have helped a lot here (note this was the only argument that
wasn't documented):

copy: destRectangle from: sourcePt in: sourceImage rule: combinationRule
    "Copy into the destination rectangle destRectangle in the
    receiver those bits in the sourceImage starting at position sourcePt,
    according the combination rule rule.  The depths of source and
destination
    must be the same.  See:
       
ByteArray>>copyBitsStride:width:atX:y:from:stride:width:atX:y:width:height:rule:

    for documentation on combinationRule."

Similar references in the other handful of methods in Image that use bitblt.


2) BrowserTabApplication>>eraseBackgroundOnImage: has the rules listed.  
There are several other methods like this...look for senders of
copy:from:in:rule: to get a big list of them.  This is an inappropriate
copy-and-paste of a comment although I don't expect new/changed BitBlt
rules any time soon ;-)

I'm disappointed to see this rejected, the more that I think about it,
because this is an area where Cincom Smalltalk needs significant work so
a little prompting to fix up a couple of poorly documented API methods
seems worthwhile.

David

_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc

David,

Ahhhh, but there are more references that could use some improvement.
Hmmmm... to some extent, I feel a bit towards not duplicating the list
because my concern would be to enter into a slippery slope where
basically any direct or indirect sender has to document what the drawing
rules are.  Some of my bias I am sure is that the 16 rules represent all
the possible combinations of two bits in either of their states, so
maybe it comes off as a more or less natural thing to me (once I see the
list at least once, of course), and so I do not initially feel towards
putting the list in more than one place.

I have not taken a look yet, but if there are indeed duplications of the
list then well... I will take a look at it and see what.  I'll reopen
the AR and assign it to myself.

Andres.

-----Original Message-----
From: C. David Shaffer [mailto:[hidden email]]
Sent: Tuesday, January 06, 2009 12:12 PM
To: Valloud, Andres; vwnc
Subject: Re: [vwnc] In-image documentation bug in VW7.6

Valloud, Andres wrote: Ouch :-)  Rejection comes hard to me :-) :-)

FWIW there were two parts to the bug report:

1) Image>>copy:from:in:rule:  -- I'll give you this one since it is only
a three step process (view copy:from:in:rule: implementors,
implementors) to get the the docs on rule.  Some comment hyperlinking
would have helped a lot here (note this was the only argument that
wasn't documented):

copy: destRectangle from: sourcePt in: sourceImage rule: combinationRule
    "Copy into the destination rectangle destRectangle in the
    receiver those bits in the sourceImage starting at position
sourcePt,
    according the combination rule rule.  The depths of source and
destination
    must be the same.  See:
       
ByteArray>>copyBitsStride:width:atX:y:from:stride:width:atX:y:width:heig
ht:rule:

    for documentation on combinationRule."

Similar references in the other handful of methods in Image that use
bitblt.


2) BrowserTabApplication>>eraseBackgroundOnImage: has the rules listed.

There are several other methods like this...look for senders of
copy:from:in:rule: to get a big list of them.  This is an inappropriate
copy-and-paste of a comment although I don't expect new/changed BitBlt
rules any time soon ;-)

I'm disappointed to see this rejected, the more that I think about it,
because this is an area where Cincom Smalltalk needs significant work so
a little prompting to fix up a couple of poorly documented API methods
seems worthwhile.

David


_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc

Perhaps the best place to put these is not in the method comment but in a separate documentation method.  Then you could just say (in the various places) "See ByteArray class>>bitbltDocumentation".

I know this wouldn't be too easy, but it seems as if we need to find a way to be able to represent this documentation in the comments and have those references hyperlink in the browser to where things really are.  Maybe we need a browser highlighting URL, something like "browse://<class>/method", that can be recognized in the hyperlink highlighting capabilities.  I do realize that would complicate method rename and refactoring in case there are browser references to the method.

Just a thought.

Cheers!
 
Tom Hawker
--------------------------
Senior Framework Developer
--------------------------
Home +1 (408) 274-4128
Office +1 (408) 576-6591
Mobile +1 (408) 835-3643
 
-----Original Message-----
From: [hidden email] [mailto:[hidden email]] On Behalf Of Valloud, Andres
Sent: Tuesday, January 06, 2009 1:15 PM
To: vwnc
Subject: Re: [vwnc] In-image documentation bug in VW7.6

David,

Ahhhh, but there are more references that could use some improvement.
Hmmmm... to some extent, I feel a bit towards not duplicating the list
because my concern would be to enter into a slippery slope where
basically any direct or indirect sender has to document what the drawing
rules are.  Some of my bias I am sure is that the 16 rules represent all
the possible combinations of two bits in either of their states, so
maybe it comes off as a more or less natural thing to me (once I see the
list at least once, of course), and so I do not initially feel towards
putting the list in more than one place.

I have not taken a look yet, but if there are indeed duplications of the
list then well... I will take a look at it and see what.  I'll reopen
the AR and assign it to myself.

Andres.

-----Original Message-----
From: C. David Shaffer [mailto:[hidden email]]
Sent: Tuesday, January 06, 2009 12:12 PM
To: Valloud, Andres; vwnc
Subject: Re: [vwnc] In-image documentation bug in VW7.6

Valloud, Andres wrote: Ouch :-)  Rejection comes hard to me :-) :-)

FWIW there were two parts to the bug report:

1) Image>>copy:from:in:rule:  -- I'll give you this one since it is only
a three step process (view copy:from:in:rule: implementors,
implementors) to get the the docs on rule.  Some comment hyperlinking
would have helped a lot here (note this was the only argument that
wasn't documented):

copy: destRectangle from: sourcePt in: sourceImage rule: combinationRule
    "Copy into the destination rectangle destRectangle in the
    receiver those bits in the sourceImage starting at position
sourcePt,
    according the combination rule rule.  The depths of source and
destination
    must be the same.  See:
       
ByteArray>>copyBitsStride:width:atX:y:from:stride:width:atX:y:width:heig
ht:rule:

    for documentation on combinationRule."

Similar references in the other handful of methods in Image that use
bitblt.


2) BrowserTabApplication>>eraseBackgroundOnImage: has the rules listed.

There are several other methods like this...look for senders of
copy:from:in:rule: to get a big list of them.  This is an inappropriate
copy-and-paste of a comment although I don't expect new/changed BitBlt
rules any time soon ;-)

I'm disappointed to see this rejected, the more that I think about it,
because this is an area where Cincom Smalltalk needs significant work so
a little prompting to fix up a couple of poorly documented API methods
seems worthwhile.

David


_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc


IMPORTANT NOTICE
Email from OOCL is confidential and may be legally privileged.  If it is not intended for you, please delete it immediately unread.  The internet cannot guarantee that this communication is free of viruses, interception or interference and anyone who communicates with us by email is taken to accept the risks in doing so.  Without limitation, OOCL and its affiliates accept no liability whatsoever and howsoever arising in connection with the use of this email.  Under no circumstances shall this email constitute a binding agreement to carry or for provision of carriage services by OOCL, which is subject to the availability of carrier's equipment and vessels and the terms and conditions of OOCL's standard bill of lading which is also available at http://www.oocl.com.

_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc

IMHO, the problem with the comments is really a code smell telling us that
using Integers for encoding the rules is not appropriate. Looking at the
senders of Image>>copy:from:in:rule:, there are only three places where the
rule is actually an Integer literal, eraseBackground: and
eraseBackgroundOnImage:. In all other methods, the rule is obtained by
sending a message to the class RasterOp, e.g. "RasterOp over". However,
RasterOp does not implement methods for all legal values of the rule
parameter.

I suggest implementing a class Graphics.CombinationRule, with appropriately
named class side methods for accessing CombinationRule instances (which may
be cached in shared variables). RasterOp would delegate its "mode constants"
methods to CombinationRule. The CombinationRule instances can store their
Integer code in an instance variable, which will be accessed in places where
it has to be passed to a primitive.

Advantages: Instead of having to document the meaning of the combinationRule
parameters all over the place, it's sufficient to mention that it's an
instance of CombinationRule, which can have a class comment describing the
meaning of the different instances in a central place. The problem of
possibly using an illegal Integer value (23, -17, or whatever) goes away,
and the legal values only have to be documented once in the CombinationRule
class comment.

Cheers,
Joachim Geidel

Am 06.01.09 22:14 schrieb Valloud, Andres:



_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc