clj-commons / etaoin

Pure Clojure Webdriver protocol implementation
https://cljdoc.org/d/etaoin
Eclipse Public License 1.0
912 stars 95 forks source link

Consider adding `fill-active-human` #629

Closed dgr closed 2 months ago

dgr commented 2 months ago

Problem/Opportunity Currently, Etaoin has a fill-active API for filling in the active element. This is convenient because it automatically queries for the active element. Etaoin also has fill-human and fill-human-el APIs for filling a queried or specified element with human-like behavior (delays, typos, etc.). There is no convenience API for filling the active element while using human-like behavior, however.

Proposed Solution Add a new API, fill-active-human that simply queries for the active element and then calls fill-human-el. Parameters would roughly match fill-human / fill-human-el, but without the query or element to fill (since that will be provided by the active element).

Alternative Solutions The alternative is simply to leave things the way they are. fill-active-human is strictly a convenience API offering no fundamentally new functionality that cannot be achieved less-conveniently using existing APIs.

Action I can submit a PR if this sounds like good functionality.

lread commented 2 months ago

Yeah, if this would be useful to you and others, Dave, feel free to submit a PR!

onetom commented 1 month ago

human- as a prefix would sound better, imho. fill-active-human sounds a bit like joda speak... or as if some "active human" would be being filled as a result of calling this function...

without being familiar with the implementation, i would even expect to just have a single as-human modifier macro, which would just do some dynamic binding to affect the how the fill function works.

dgr commented 1 month ago

IMO, there's no great solution here. First, I tried to make this fit in with Etaoin's existing naming scheme. Etaoin uses -human at the end of something to indicate that it uses human-like delays and mistakes to fill something in. Another reasonable, though less optimal, IMO, choice would have been fill-human-active, which I considered but rejected because of the way the other functions were named. That eliminates some of the "active human" issue. If somebody has real heartburn and wants to suggest an alternative name, I'd be open to that. I don't think the modifier macro is a good idea. My general philosophy is that functions should just do what they say they do, without modifying behavior based on dynamic global state.

lread commented 1 month ago

Sounds good, I don't think we want to rename/rework existing working functionality too much. For example, there is the somewhat oddly named wait-has-text-everywhere, which is probably due to English not being the first language of the author, but the docstring explains its intent.

dgr commented 1 month ago

After staring at this some more, I think we should change names from fill-active-human to fill-human-active. I think that actually fits in better with the overall Etaoin naming scheme.

onetom commented 1 month ago

fill-human-active has the same un-english-ness and ambiguity to it as fill-active-human, though, since we are not filling any humans here. human is just an adjective of fill.

but english is my 2nd language too, so my gut-feel is probably not the most representative in such matters.

First, I tried to make this fit in with Etaoin's existing naming scheme

it's not necessarily a good principle, if it became apparent over time, that the scheme was not the best... im all for backwards compatibility, but also for intuitiveness.