allwpilib icon indicating copy to clipboard operation
allwpilib copied to clipboard

[cmd] Add `andThenWaitUntil`, `andThenWaitSeconds`, `andThenWaitTime` command decorators

Open katzuv opened this issue 1 year ago • 11 comments

I can add Python later and try to add C++ implementation. English is not my first language, so I'd appreciate suggestion for improving the docs.

katzuv avatar Oct 11 '24 12:10 katzuv

This PR modifies commands. Please open a corresponding PR in Python Commands and include a link to this PR.

github-actions[bot] avatar Oct 11 '24 12:10 github-actions[bot]

thenAwait or andThenAwait?

katzuv avatar Oct 11 '24 19:10 katzuv

/format

katzuv avatar Oct 13 '24 09:10 katzuv

Maybe (and)ThenWaitUntil()?

Starlight220 avatar Oct 13 '24 09:10 Starlight220

Maybe (and)ThenWaitUntil()?

I think "await" == "wait until", no? The "and" can be added, I don't know what the preferences are for more than two words for decorators.

katzuv avatar Oct 13 '24 09:10 katzuv

/format

The format command won't work cause the PR is from the main branch of your form

spacey-sooty avatar Oct 13 '24 09:10 spacey-sooty

The format command won't work cause the PR is from the main branch of your form

I created a new branch in my repo (then-wait-await) but GitHub lets me change the destination branch only. Can someone with higher permissions edit the PR? Otherwise I'll open a new one, if the formatting is incorrect.

katzuv avatar Oct 13 '24 10:10 katzuv

Java format is failing, locally you can just run ./gradlew spotlessApply which should fix it

spacey-sooty avatar Oct 13 '24 10:10 spacey-sooty

Personally I'd like these to be (and)Then... followed by the named of the Commands factory (e.g., thenWaitUntil(BooleanSupplier) or andThenWaitUntil(BooleanSupplier)), since that should make it easier for people to remember.

KangarooKoala avatar Oct 14 '24 02:10 KangarooKoala

I'm not a fan of adding these.

andThenWaitUntil(foo()) andThen(waitUntil(foo()))

andThenWaitSeconds(foo()) andThen(waitSeconds(foo()))

You save two characters.

rzblue avatar Oct 14 '24 15:10 rzblue

I'm not a fan of adding these.

andThenWaitUntil(foo()) andThen(waitUntil(foo()))

andThenWaitSeconds(foo()) andThen(waitSeconds(foo()))

You save two characters.

True. I did imagine them more concise, with it initially being thenAwait. But it does save you the Commands. part.

katzuv avatar Oct 14 '24 15:10 katzuv