Convert DateComponents code parsing code to use Optional based return values rather than out-parameters
Created attachment 402408 [details] Patch
Created attachment 402409 [details] Patch
Created attachment 402410 [details] Patch
Created attachment 402415 [details] Patch
Created attachment 402420 [details] Patch
Created attachment 402425 [details] Patch
Comment on attachment 402425 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402425&action=review > Source/WebCore/html/BaseDateAndTimeInputType.cpp:81 > + return !value.isEmpty() && parseToDateComponents(value) == WTF::nullopt; I think !parse reads better than parse == nullopt. But I suppose others might argue that the ! one looks like itβs about 0; not for me. (As an aside, I hope we change our style to allow "== 0" at some point.) > Source/WebCore/html/BaseDateAndTimeInputType.h:58 > + virtual Optional<DateComponents> setMillisecondToDateComponents(double) const = 0; What does the word "set" refer to in this (already existing, not new to this patch) function name? > Source/WebCore/platform/DateComponents.cpp:116 > + const CharacterType* current = src + parseStart; > + const CharacterType* end = current + parseLength; I just woulda done auto for these. > Source/WebCore/platform/DateComponents.cpp:201 > + return createFromString(source, [] (auto& date, const auto* src, auto length, auto& end) { Not a big fan of the use of the string "src" here. Also I think just auto src would do, no need for const auto*.
Comment on attachment 402425 [details] Patch Even though I had comments I think I will set commit-queue+, probably good to land this as-is.
Committed r263331: <https://trac.webkit.org/changeset/263331> All reviewed patches have been landed. Closing bug and clearing flags on attachment 402425 [details].
<rdar://problem/64578366>
Reverted r263331 for reason: This commit was causing 50+ iOS debug tests to crash. Committed r263349: <https://trac.webkit.org/changeset/263349>
Created attachment 403461 [details] Patch
(In reply to Sam Weinig from comment #12) > Created attachment 403461 [details] > Patch Now with the missing empty string check added back.
Committed r263900: <https://trac.webkit.org/changeset/263900> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403461 [details].