Provide String::indexOf for a char* needle by paulocsanz · Pull Request #7706

Provide String::indexOf for a char* needle by paulocsanz · Pull Request #7706 · esp8266/Arduino

paulocsanz changed the title ~~Provide String::indexOf for char* needle~~ Provide String::indexOf for a char* needle

Nov 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for not only filing an issue, but proposing a solution too!

You mentioned progmem in your issue and changing to strstr_Pwill make it "just work" with RAM or PROGMEM strings.

strstr_P documentation says only s2 must be from PROGMEM, so we are
safe.

I've went ahead and implemented the changes. Should I change all strstr calls to strstr_P then? The documentation doesn't say anything about passing a needle to strstr_P that does not live in program memory.

But again I'm not an expert in this ecosystem.

Btw did I break anything? The CI seems to have errored (I may have hurt their feelings by force pushing too much lol).

Btw did I break anything? The CI seems to have errored (I may have hurt their feelings by force pushing too much lol).

Yes

(click on "details" on any CI line)

edit: It is quite never needed to force-push. Pushing is OK, pulling is needed before pushing if the branch has been updated from github. That way, we have history. The history is anyway squashed at merge time on this repository.

Thank you @d-a-v! I've just fixed it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the single const change noted below, LGTM. Thx!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx! LGTM.