D21266: [WIP] Improve documentation of area classes

Albert Astals Cid noreply at phabricator.kde.org
Sat Jun 22 14:30:24 BST 2019


aacid accepted this revision.
aacid added a comment.
This revision is now accepted and ready to land.


  There's a few typos and i think i don't really agree with the "rectangles should not have negative width/height" wording, they don't make much sense, but on principle don't tell people what they can or can not do.
  
  But let's commit this, i think it's not worse of what we have in general.
  
  Thanks :)

INLINE COMMENTS

> area.h:94
> + * (This is what TextPage actually does.)
> + * Mouse click and release events are given in page coordinates (400, 180) and (600, 450),
> + * while the page has a size of 800x600.

click->press

> area.h:97
> + *
> + * If you want to seach all text between the mouse click and release event,
> + * you need their normalized coordinates.

seach -> search, click -> press

> area.h:214
> +         * @note
> +         * The coordinates for @p left and @p top must be lower than
> +         * @p right and @p bottom, respectively, to avoid negative width or height.

what if they want it to be negative for some reason? Does anything break? i mean the lines above clearly say left, top, and bottom.

> area.h:224
> +         * @note
> +         * The rectangle must have positive width and height.
> +         * You can use e. g. QRect::normalize() to ensure this.

same as above, does it really need to have a positive width and height?

> davidhurka wrote in area.h:108
> It would make sense to make it non static, because the first parameter is a NormalizedPoint (consists of x and y, xScale and yScale don’t scale the point).
> 
> Anyway, OKULARCORE_EXPORT is a reason to keep it static, right?

it's exported api, so yes let's not change it, and less in a "improve documentation" MR ;)

> davidhurka wrote in area.h:128
> TextEntity provides example usage, so someone who wonders why this class exists will be happy.
> 
> Throwing in this link, because it mentions examples.
> https://community.kde.org/Guidelines_and_HOWTOs/API_Documentation#Writing_APIDOX_in_New_Code
> 
> I will remove the “glyphs, words, line...”, that’s too specific.

i still don't like the "is used" wording, it's like binds it forever, if you really want to reference a class (can't people use grep or find all in an IDE), i'd prefer a wording like "for example is used in" or similar

REPOSITORY
  R223 Okular

BRANCH
  improve-area-classes-documentation

REVISION DETAIL
  https://phabricator.kde.org/D21266

To: davidhurka, #okular, aacid
Cc: aacid, okular-devel, fbampaloukas, joaonetto, tfella, ngraham, darcyshen
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20190622/3853106b/attachment.html>


More information about the Okular-devel mailing list