Comment 7 for bug 518011

Revision history for this message
RaiMan (raimund-hocke) wrote :

Since I really like the approach of Sikuli, its a pleasure for me to give some input for further development. So you are welcome.

on the draft itself:

- You should give every function its own line. Its clearer and easier to scan.

- dragdrop/dragdropAll should read
dragDrop( Pattern/String/Match/Matches, Pattern/String/Match/Matches/List or Tuple of 2 absolute coordinates [x,y], [delay1], [delay2] )
dragDropAll( Pattern/String/Match/Matches, Pattern/String/Match/Matches/List or Tuple of 2 absolute coordinates [x,y], [delay1], [delay2], [delay between each dragDrop] ) # only dragDropAll(multiples, single) or dragDropAll(single, multiples) are accepted
to avoid the question: Whats special with the second form?

- In the intro, you should state once, that every timing parm (timeout, delay) have to be specified as seconds and can be a decimal. so you can omit this info at the function itself.
This in mind: wait, untilNotExist, waitVanish should read: ... ( [Pattern/String/Match/Matches], timeout )

- with ...clickAll/hoverAll it should read [delay between the single actions]

comments on the content itself:

- with dragDrop/dragDropAll: what do delay1/delay2 do? I guess internally, first the finds are processed. If for source and target at least one match is found, the action click-hold, mouse-move, click-release is processed accordingly. So I guess: click-hold, delay1, mouse-move, delay2, click-release???

- with dragDropAll: the delay parms should be in this order: [delay between each dragDrop], [delay1], [delay2] because I think the between-delay is used more often

- Actions on Subregions directly (actions: single-clicks, type, paste):
idea: Subregion should be a valid parm for these actions e.g. click(Subregion or list/tuple with 4 (region) or 2 (point) values). why?: convenience and performance (without this: click(capture(x,y,w,h)) which has to go thru find and produces an image)

Here I have a problem with the API doc:
Intro now:
Subregion limits the region that find() works on. It will support all action commands as well as spatial operators.

Class Subregion now:
all the actions are listed, what should be understood as Subregion.click() being possible. But with the given parms as Pattern/String/Match/Matches, this makes no sense, since it is already possible with Subregion.inside().click(). I think this is straightforward, because it implies, that with the spatials always something has to be found.

Conclusion:
So I think its clearer to allow action(Pattern/String/Match/Matches/Subregion, ...)
In the Intro you could say:
Subregion limits the region that find() works on. It will support all action commands via the spatial operators. As a convenience Subregion can be a parameter for all actions that accept Pattern/String/Match/Matches and can always be substituted by a list or tuple with 4 values (a region) or 2 values (a point).
Class Subregion: don't list the actions

- as a consequence Subregion/Match/Matches.find/findAll() should not be allowed, since its possible with x.inside().find/findAll() already.

- NEW class Subregions: since Match differs from Subregion only in knowing its content plus similarity, with this overall revision it is only consequent to implement Subregions as a counterpart to Matches.
At least this should be possible:
construction: Subregions(), Subregions(Match/Matches), Subregion(Subregion/Subregions)
usage: for s in Subregion, Subregion.add(Match/Matches/Subregion/Subregions)

- NEW Matches and Subregions should have a sort based on their [x,y] with the two choices HORIZONTAL (left to right and then top to bottom) and VERTICAL (top to bottom then left to right) starting in the upper left. (e.g. Matches.sorth() and Matches.sortv()). This would make some scripts much easier, since you could avoid additional python.