Swap definitions of isWhitespace and isSpace

[per1234]

Previously, the behavior of isWhitespace did not match what was stated in the documentation:

Analyse if a char is a white space, that is space, formfeed ('\f'), newline ('\n'), carriage return ('\r'), horizontal tab ('\t'), and vertical tab ('\v')). Returns true if thisChar contains a white space.

Previously, the behavior of isSpace did not match what was stated in the documentation:

Analyse if a char is the space character. Returns true if thisChar contains the space character.

A strict reading of the documentation for isSpace would indicate that it should only match on ASCII value 32 (true space). The change made here will cause it to match only a true space or a tab.

---

I realize this is a breaking change. It is the action recommended in the issue report:
arduino/Arduino#7041 (comment)

The alternative is to leave the code as-is and correct the documentation, which I am happy to do if the decision goes that way.

There is also the question of how isSpace should work. I can make it match on space, and not on tab if that's preferable. If left as it currently is in this PR, the documentation will need to be updated (once the updated code has been released) to indicate that it also matches on tab.

I'm submitting the PR to this repo as it seems that ArduinoCore-API should now act as the model for the non-architecture specific code in other cores until the time comes for them to transition to using ArduinoCore-API directly. If accepted here, I can submit the equivalent pull requests to the other core repositories.

---

Fixes arduino/Arduino#7041

Reference: http://forum.arduino.cc/index.php?topic=597290

---

Demonstration sketch:

void setup() {
  Serial.begin(9600);
  while (!Serial) {}
  const char chars[] = {'\f', '\n', '\r', '\v', '\t', ' ', 'a'};
  for (byte charCounter = 0; charCounter < sizeof(chars); charCounter++) {
    Serial.print("isWhitespace(");
    Serial.print((byte)chars[charCounter]);
    Serial.print(") == ");
    Serial.println(isWhitespace(chars[charCounter]) ? "true" : "false");
    Serial.print("isSpace(");
    Serial.print((byte)chars[charCounter]);
    Serial.print(") == ");
    Serial.println(isSpace(chars[charCounter]) ? "true" : "false");
  }
}

void loop() {}

[facchinm]

Thanks for taking time to tackle this! I'd prefer to change the documentation (the actual behavior "makes sense" for me). But we should reach a bigger consensus before merging.
@tigoe @sandeepmistry @cmaglie @mbanzi any thoughts?

[tigoe]

Seems to make sense to change to me. The original idea should have been as the documentation says. T

…

On Wed, Feb 13, 2019, 3:27 AM Martino Facchin ***@***.***> wrote: Thanks for taking time to tackle this! I'd prefer to change the documentation (the actual behavior "makes sense" for me). But we should reach a bigger consensus before merging. @tigoe <https://github.com/tigoe> @sandeepmistry <https://github.com/sandeepmistry> @cmaglie <https://github.com/cmaglie> @mbanzi <https://github.com/mbanzi> any thoughts? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXOn2I8wzJwAGIuDujEMIefbqLsQJ7bks5vM8yBgaJpZM4a4SGZ> .

[per1234]

Documentation for isspace definitely indicates that it's intended to check for white-space characters:

• http://www.cplusplus.com/reference/cctype/isspace/
• https://en.cppreference.com/w/cpp/string/byte/isspace

I didn't find anything more official to back that. Here's a quote from the closest draft to the C++11 standard, section 2.5, paragraph 2:

or white-space characters (space, horizontal tab, new-line, vertical tab, and form-feed)

That's not a formal definition of white-space, but it does make it seem that a function named isWhitespace would ideally match on more than only space and tab.

That said, I am favorable to the argument that long-established core functions should not suddenly change their behavior, even if that behavior doesn't match the function name so well. I'm just not convinced by the argument that the current behavior "makes sense".

[pnndra]

although i understand that common naming would suggest that the code fix is the right thing on the other hand it may potentially break existing applications so after a quick consultation within Arduino we prefer to fix documentation rather than change code that could break existing applications.

[per1234]

Good call. I will make the documentation change.