Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
23480035:
Added examples to String docs. (Closed)
|
Created:
7 years, 3 months ago by shailentuli Modified:
7 years, 2 months ago CC:
reviews_dartlang.org Visibility:
Public. |
DescriptionAdded examples to String docs.
R=floitsch@google.com, lrn@google.com
Committed: https://code.google.com/p/dart/source/detail?r=27984
Patch Set 1 #
Total comments: 109
Patch Set 2 : Updates based on Mem's comments. #
Total comments: 46
Patch Set 3 : Numerous post LGTM fixes. #
Total comments: 5
Messages
Total messages: 15 (0 generated)
PTAL
ptal.
some editing things. mem https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.dart File sdk/lib/core/string.dart (right): https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:8: * A class for working with a sequence of characters. how about just "A sequence of characters." One of the guidelines is to make "the first words meaningful" and to avoid things like "support for" and "an api" for.... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:12: * written using triple quotes. These are all valid Dart strings: The following are all ... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:28: * Strings are immutable. While you cannot change a string, you can perform an Although you cannot... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:32: * var newString = string.toUpperCase(); Do you need // 'Dart is fun' // 'DART IS FUN' ?? Also use an exclamation mark! 'Dart is fun!' https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:34: * You can use the `+` operator to concatenate strings: You can use the plus (`+`) operator ... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:38: * You can also use string literals for concatenation: You can also use adjacent string literals ... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:43: * within strings. The curly braces can be omitted when evaluating identifiers: You can use `${}` ... also for interpolating -> to interpolate https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:49: * accessible through the [codeUnitAt] or the [codeUnits] members: --> accessible with [codeUnits] or [codeUnitAt]: (also put in same order as snippet below) https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:55: * The string representation of the code units is accessible through the index The string representation -> the character representation ?? https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:62: * terminology to Go we use the name 'rune' for an integer representing a Remove reference to Go....also reword... A 'rune' is an integer representing a Unicode code point. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:64: * property: Use the [runes] property to get the runes of a string. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:68: * For a character outside the Basic Multilingual Plane (plane 0) that is I feel like this para is too much for the class-level section. Move to [runes]. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:68: * For a character outside the Basic Multilingual Plane (plane 0) that is you haven't defined the basic multilingual plane https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:73: * and using `runes` returns their combined value: Remove 'using', link to codeUnits --> [codeUnits] returns the surrogate pair, and `runes` returns their combined value https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:80: * String. Extending or implementing String is a compile-time error. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:84: * See [StringBuffer] to efficiently build a string incrementally. See delete 'incrementally' https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:88: * for String examples and recipes. Link to Dart Up & Running, language tour, and library tour Also...make it into a bulleted list. https://www.dartlang.org/docs/dart-up-and-running/contents/ch02.html#strings https://www.dartlang.org/docs/dart-up-and-running/contents/ch03.html#ch03-str... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:100: * If it is greater than 16-bits, it is decomposed into a surrogate pair: If a char-code value is greater than 16-bits, https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:112: * represented by a single UTF-16 code unit. Otherwise the [length] is 2 and Reverse the sense of the first sentence. Add comma after Otherwise. If the [charCode] can be represented by a single UTF-16 code unit, then the new string contains a single code unit. Otherwise, https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:117: * one half of a surrogate pair. It is allowed is passive. How about the following instead: You should avoid creating a String with half of a surrogate pair. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:150: * of [runes] might be less, if the string contains characters outside might be less -> might be fewer https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:163: * Returns whether the two strings are equal. Returns true if the two strings are equal. False, otherwise. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:166: * Equivalently (for strings that are well-formed UTF-16) it compares each For strings that are well-formed UTF-16, it compares ... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:173: * In the first string the "é" is encoded as a single unicode code unit (also The first string encodes "é" as a single unicode code unit (also a single rune). https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:174: * a single rune), whereas the second string encodes it as "e" with the The second string encodes "é" as the letter "e" and the combining accent character "◌́". https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:180: * Returns whether this string ends with [other]. For example: Return true if this string ... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:187: * Returns whether this string starts with a match of [pattern]. Returns true if this string... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:193: * If [index] is provided, instead checks if the substring starting instead -> this method https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:199: * It is an error if [index] is negative or greater than [length]. An error occurs if ... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:201: * A [RegExp] containing '^' will not match if the [index] is greater than will -> does https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:201: * A [RegExp] containing '^' will not match if the [index] is greater than Here single quotes are used to delineate a character '^'. Other places use double quotes "é". Prolly should make that consistent. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:211: * Returns the first position of a match of [pattern] in this string, Returns the position of the first match of [pattern] ... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:218: * Returns -1 if a match is not found: Returns -1 if no match is found. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:222: * It is an error if [start] is negative or greater than [length]. An error occurs if ... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:227: * Returns the last position of a match [pattern] in this string, searching Returns the position of the last match of [pattern] ... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:238: * It is an error if start is negative or greater than [length]. An error occurs ... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:243: * Returns whether this string is empty. Returns true if this string is empty. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:248: * Returns whether this string is not empty. Returns true if this string is not empty. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:257: * A sequence of strings can be concatenated by using [Iterable.join]: Use [Iterable.join] to concatenate a sequence of strings. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:265: * Returns a substring of this string in the given range. Returns the substring of this string that extends from [startIndex] (inclusive) to [endIndex] (exclusive). https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:277: * If the string contains leading or trailing whitespace a new string with no whitespace --> whitespace, https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:282: * Otherwise, the string itself is returned: the string itself --> the original string https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:309: * Returns whether this string contains a match of [other]: Returns true if this string contains a match of [other]: https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:315: * If [startIndex] is provided, only matches at or after that index ..., this method only considers matches at or after that index. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:321: * It is an error if [startIndex] is negative or greater than [length]. An error occurs if ... https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:326: * Returns a new string where the first occurence of [from] in this string where -> in which https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:334: * Replaces all substrings matching [from] with [replace]. matching -> that match https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:336: * Returns a new string where the non-overlapping substrings that match where -> in which https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:349: * Replace all substrings matching [from] by a string computed from the match. matching -> that match https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:361: * The function defined below converts each word in some text to 'pig latin' some text -> a string https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:373: * Splits the string around matches of [pattern]. Returns around -> at https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:390: * Unless it guaranteed that the string is in the basic multilingual plane what is "it" here? I'm not sure what this is saying...is this right? You should [map] the runes if the string is not in the basic multilingual plane. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:399: * Splits the string on the [pattern], then converts each matched and on -> at https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:400: * unmatched part. converts it to what? https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:400: * unmatched part. How about this: Splits the string based on [pattern], converts both the matched and unmatched parts, and rejoins the parts into a new string. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:426: * Returns an iterable of Unicode code-points of this string. iterable -> [Iterable] https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:435: * If this string is not already all lower case, returns a new string Put the verb first.... Converts all characters in this string to lower case. If the string is already in all lower case, this method returns this. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:445: * If this string is not already all upper case, returns a new string same as toLowerCase.
Kathy, can you take a look?
Kathy, can you take a look?
https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.dart File sdk/lib/core/string.dart (right): https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:8: * A class for working with a sequence of characters. On 2013/09/05 23:09:09, mem wrote: > how about just "A sequence of characters." > > One of the guidelines is to make "the first words meaningful" and to avoid > things like "support for" and "an api" for.... Punting to kathy. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:12: * written using triple quotes. These are all valid Dart strings: On 2013/09/05 23:09:09, mem wrote: > The following are all ... Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:28: * Strings are immutable. While you cannot change a string, you can perform an On 2013/09/05 23:09:09, mem wrote: > Although you cannot... Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:32: * var newString = string.toUpperCase(); On 2013/09/05 23:09:09, mem wrote: > Do you need > // 'Dart is fun' > // 'DART IS FUN' > ?? > > Also use an exclamation mark! 'Dart is fun!' Not really. I'm just showing assignment. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:34: * You can use the `+` operator to concatenate strings: On 2013/09/05 23:09:09, mem wrote: > You can use the plus (`+`) operator ... Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:38: * You can also use string literals for concatenation: On 2013/09/05 23:09:09, mem wrote: > You can also use adjacent string literals ... Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:43: * within strings. The curly braces can be omitted when evaluating identifiers: On 2013/09/05 23:09:09, mem wrote: > You can use `${}` ... > > also > for interpolating -> to interpolate Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:49: * accessible through the [codeUnitAt] or the [codeUnits] members: On 2013/09/05 23:09:09, mem wrote: > --> accessible with [codeUnits] or [codeUnitAt]: > > (also put in same order as snippet below) Leaving Florian's ordering. Re-ordered my example. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:55: * The string representation of the code units is accessible through the index On 2013/09/05 23:09:09, mem wrote: > The string representation -> the character representation > > ?? No, better as string. String is well understood, character is a bit fuzzy in Dart. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:62: * terminology to Go we use the name 'rune' for an integer representing a On 2013/09/05 23:09:09, mem wrote: > Remove reference to Go....also reword... > A 'rune' is an integer representing a Unicode code point. Keeping Florian's language. He wants the Go reference. I think it is good to have it there. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:68: * For a character outside the Basic Multilingual Plane (plane 0) that is On 2013/09/05 23:09:09, mem wrote: > I feel like this para is too much for the class-level section. Move to [runes]. No, keeping it. This runes vs. codeUnits thing applies to several methods, and a class level discussion is warranted. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:68: * For a character outside the Basic Multilingual Plane (plane 0) that is On 2013/09/05 23:09:09, mem wrote: > you haven't defined the basic multilingual plane That's OK. This is not a Dart concept. Its a big topic, and users are better off understanding this very well from other sources. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:73: * and using `runes` returns their combined value: On 2013/09/05 23:09:09, mem wrote: > Remove 'using', link to codeUnits > > --> [codeUnits] returns the surrogate pair, and `runes` returns their combined > value Keeping it as is, because I don't want the sentence to start with a lc letter. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:80: * String. On 2013/09/05 23:09:09, mem wrote: > Extending or implementing String is a compile-time error. Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:84: * See [StringBuffer] to efficiently build a string incrementally. See On 2013/09/05 23:09:09, mem wrote: > delete 'incrementally' No, keeping it. 'Build a string' by itself reads poorly to me. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:88: * for String examples and recipes. On 2013/09/05 23:09:09, mem wrote: > Link to Dart Up & Running, language tour, and library tour > > Also...make it into a bulleted list. > > https://www.dartlang.org/docs/dart-up-and-running/contents/ch02.html#strings > https://www.dartlang.org/docs/dart-up-and-running/contents/ch03.html#ch03-str... Linking to the Library tour. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:100: * If it is greater than 16-bits, it is decomposed into a surrogate pair: On 2013/09/05 23:09:09, mem wrote: > If a char-code value is greater than 16-bits, Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:112: * represented by a single UTF-16 code unit. Otherwise the [length] is 2 and On 2013/09/05 23:09:09, mem wrote: > Reverse the sense of the first sentence. Add comma after Otherwise. > > If the [charCode] can be represented by a single UTF-16 code unit, then the new > string contains a single code unit. Otherwise, Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:117: * one half of a surrogate pair. On 2013/09/05 23:09:09, mem wrote: > It is allowed is passive. How about the following instead: > > You should avoid creating a String with half of a surrogate pair. Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:150: * of [runes] might be less, if the string contains characters outside On 2013/09/05 23:09:09, mem wrote: > might be less -> might be fewer Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:166: * Equivalently (for strings that are well-formed UTF-16) it compares each On 2013/09/05 23:09:09, mem wrote: > For strings that are well-formed UTF-16, it compares ... Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:173: * In the first string the "é" is encoded as a single unicode code unit (also On 2013/09/05 23:09:09, mem wrote: > The first string encodes "é" as a single unicode code unit (also a single rune). Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:174: * a single rune), whereas the second string encodes it as "e" with the On 2013/09/05 23:09:09, mem wrote: > The second string encodes "é" as the letter "e" and the combining accent > character "◌́". Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:180: * Returns whether this string ends with [other]. For example: On 2013/09/05 23:09:09, mem wrote: > Return true if this string ... Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:187: * Returns whether this string starts with a match of [pattern]. On 2013/09/05 23:09:09, mem wrote: > Returns true if this string... Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:193: * If [index] is provided, instead checks if the substring starting On 2013/09/05 23:09:09, mem wrote: > instead -> this method Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:199: * It is an error if [index] is negative or greater than [length]. On 2013/09/05 23:09:09, mem wrote: > An error occurs if ... Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:201: * A [RegExp] containing '^' will not match if the [index] is greater than On 2013/09/05 23:09:09, mem wrote: > will -> does Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:201: * A [RegExp] containing '^' will not match if the [index] is greater than On 2013/09/05 23:09:09, mem wrote: > Here single quotes are used to delineate a character '^'. Other places use > double quotes "é". > Prolly should make that consistent. Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:211: * Returns the first position of a match of [pattern] in this string, On 2013/09/05 23:09:09, mem wrote: > Returns the position of the first match of [pattern] ... Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:218: * Returns -1 if a match is not found: On 2013/09/05 23:09:09, mem wrote: > Returns -1 if no match is found. Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:222: * It is an error if [start] is negative or greater than [length]. On 2013/09/05 23:09:09, mem wrote: > An error occurs if ... Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:227: * Returns the last position of a match [pattern] in this string, searching On 2013/09/05 23:09:09, mem wrote: > Returns the position of the last match of [pattern] ... Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:238: * It is an error if start is negative or greater than [length]. On 2013/09/05 23:09:09, mem wrote: > An error occurs ... Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:257: * A sequence of strings can be concatenated by using [Iterable.join]: On 2013/09/05 23:09:09, mem wrote: > Use [Iterable.join] to concatenate a sequence of strings. Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:265: * Returns a substring of this string in the given range. On 2013/09/05 23:09:09, mem wrote: > Returns the substring of this string that extends from [startIndex] (inclusive) > to [endIndex] (exclusive). Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:277: * If the string contains leading or trailing whitespace a new string with no On 2013/09/05 23:09:09, mem wrote: > whitespace --> whitespace, Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:282: * Otherwise, the string itself is returned: On 2013/09/05 23:09:09, mem wrote: > the string itself --> the original string Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:309: * Returns whether this string contains a match of [other]: On 2013/09/05 23:09:09, mem wrote: > Returns true if this string contains a match of [other]: Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:315: * If [startIndex] is provided, only matches at or after that index On 2013/09/05 23:09:09, mem wrote: > ..., this method only considers matches at or after that index. Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:315: * If [startIndex] is provided, only matches at or after that index On 2013/09/05 23:09:09, mem wrote: > ..., this method only considers matches at or after that index. Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:321: * It is an error if [startIndex] is negative or greater than [length]. On 2013/09/05 23:09:09, mem wrote: > An error occurs if ... Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:326: * Returns a new string where the first occurence of [from] in this string On 2013/09/05 23:09:09, mem wrote: > where -> in which Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:361: * The function defined below converts each word in some text to 'pig latin' On 2013/09/05 23:09:09, mem wrote: > some text -> a string Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:390: * Unless it guaranteed that the string is in the basic multilingual plane On 2013/09/05 23:09:09, mem wrote: > what is "it" here? > > I'm not sure what this is saying...is this right? > > You should [map] the runes if the string is not in the basic multilingual plane. Using some of your's and some of Florian's language. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:399: * Splits the string on the [pattern], then converts each matched and On 2013/09/05 23:09:09, mem wrote: > on -> at Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:400: * unmatched part. On 2013/09/05 23:09:09, mem wrote: > converts it to what? Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:426: * Returns an iterable of Unicode code-points of this string. On 2013/09/05 23:09:09, mem wrote: > iterable -> [Iterable] Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:435: * If this string is not already all lower case, returns a new string On 2013/09/05 23:09:09, mem wrote: > Put the verb first.... > > Converts all characters in this string to lower case. > > If the string is already in all lower case, this method returns this. Done. https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:445: * If this string is not already all upper case, returns a new string On 2013/09/05 23:09:09, mem wrote: > same as toLowerCase. Done.
Florian, Lasse, I've started adding examples to the API docs for some of the foundational Dart classes and would like your input. The goal is to have an introduction to each class that contains some examples, and to also add examples to every non-trivial class member. I am testing these examples, and writing them in a way that makes it possible to parse the docs and generate tests automatically. It is still early, but if we can truly automate this process, we can ensure the accuracy of the API docs. PTAL. Your feedback is invaluable. In a doc that I am preparing, I am detailing what I consider best practices for writing examples in documentation. I will share that with you. Our goal should be to have API docs that have a similar look and feel across the Dart project and 100% accurate examples. PTAL.
LGTM with some comments and nits. https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... File sdk/lib/core/string.dart (right): https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:11: * written using matching single or double quotes, and mutliline strings are multiline https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:12: * written using triple quotes. The following are all valid Dart strings: If you introduce multiline strings here, you should probably already mention their peculiarity (that the remove a leading new line). https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:29: * an operation on a string and assign the result to a new string: To me "operations on strings and assign the result to new strings" sounds more natural. https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:32: * var newString = string.toUpperCase(); Please don't use "toUpperCase" in the leading intro to Strings. If I could I would remove it from strings entirely... (yes. this is a personal pet peeve). https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:43: * within strings. The curly braces can be omitted when evaluating identifiers: nit. (but no need to change the documentation): this is only true as long as the identifier does not contain a "$". https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:65: * string.runes.toList(); // [68, 97, 114, 116] That example would be more interesting if it was actually different than the codeunits. Maybe make the original string "Dart \u{100d8}". (the Dart unicode character http://www.fileformat.info/info/unicode/char/100d8/index.htm) https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:72: * and using `runes` returns their combined value: If you talk about runes here, no need to have the example above. It only becomes clear after the explanation. https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:78: * Extending or implementing String is a compile-time error. nit: this was already muddy before, but now that it's not the class that does the implementing or extending we can't say that the action itself is a compile-time error. maybe. "The String class can not be extended or implemented. Attempting to do so yields a compile-time error." https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:113: * If the [charCode] can be represented by a single ITF-16 code unit, the new UTF https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:118: * You should avoid creating a String with half of a surrogate pair. That doesn't feel like what I wanted to say. Let's just write: "It is legal to create a String with only half of a surrogate pair". (and I'm willing to negotiate on the "only"). https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:130: * invalid UTF-16 string: ... UTF-16 string, but valid Dart string: https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:133: * // These represent invalid UTF-16 strings. What are "these" ? clef[0].codeUnits is not a string. https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:152: * the Basic Multilingual Plane (plane 0). For example: At some other place you removed the "Example" part. I'm ok with keeping the "For example", though. https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:167: * For strings that are well-formed UTF-16, it compares each This sentence now makes even less sense than before. The "Equivalently" could have been "Consequently", but now the sentence almost gives a new meaning (first "code unit", then "rune"). I vote for removal. https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:181: * Returns true if this string ends with [other]. For example: ditto. At some other place you removed the "Example" introduction. https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:200: * An error occurs if [index] is negative or greater than [length]. As said in other CLs. We don't like "An error occurs". Either keep the "It is an error", or write "[index] must not be negative or greater than [length]". https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:213: * starting at [start] (inclusive). For example: ditto ("for example"). https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:223: * An error occurs if [start] is negative or greater than [length]. ditto ("An error occurs"). https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:239: * In error occurs if start is negative or greater than [length]. ditto. And "An error" not "In error". https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:258: * Use [Iterable.join] to concatenate a sequence of strings: Should also reference the StringBuffer class. https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:267: * (inclusive) to [endIndex] (exclusive). "[startIndex](inclusive)" is valid markdown but won't do, what you thought it would do(just look at the generated output). https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:283: * Otherwise, the original string itself is returned: The correct example would verify that the returned value is identical. https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:322: * An error occurs if [startIndex] is negative or greater than [length]. ditto ("An error occurs"). https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:328: * is replaced with [to]. For example: ditto ("for example"). https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:330: * '0.0001'.replaceFirst(new RegExp(r'0+'), ''); // '.0001' bad example. Why the "0+" (and not just "0") ? What's the purpose? https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:341: * 'resume'.replaceAll(new RegExp(r'e'), '\u00E9'); // 'résumé' no need for the \u. Just put "é" there. https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:362: * The function defined below converts each word in a string to 'pig latin' to simplified 'pig latin' ... ? https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:396: * the basic multilingual plane (meaning that each code unit represents a I guess you wanted to remove the old paragraph. https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:407: * The pattern is used to split the string into parts and separating matches. [pattern] https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:418: * onMatch: (m) => '*${m.group(0).toLowerCase()}*', please don't use toLowerCase/toUpperCase in examples that work with language. It won't do the right thing in other languages. https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:444: * 'abc'.toLowerCase(); // 'abc' Please add documentation that this function uses the language independent Unicode mapping and thus only works in very few languages. You can add the example of dotless I. In Turky "I".toLowerCase() should not be "i". https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:454: * 'ABC'.toUpperCase(); // 'ABC' ditto. "i".toUpperCase should be "İ" in Turkey. http://en.wikipedia.org/wiki/Dotted_and_dotless_I https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:471: throw new StateError('No elements.'); why? (not that I care).
lgtm https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart File sdk/lib/core/string.dart (right): https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart#n... sdk/lib/core/string.dart:10: * A string can be either single or multiline. Single line strings are "A string literal can be either single- or multiline." (or how do you abbreviate "single-line or multi-line"). This talks about string *literals*. String objects do not have the notion of single-line or multi-line, nor do they have any quoting. String interpolations are string literals, but they do not represent a single string. Rather they are expressions that evaluate to strings, just as "foo" + x + "bar" . https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart#n... sdk/lib/core/string.dart:40: * 'Dart ' 'is ' 'fun!'; // 'Dart is fun!' This makes it sound like the plus operator is the same as string juxtaposition. I'd rather group the plus operator with string interpolations. Something like: You can split a string literal into adjacent literal parts: 'Dart ' 'is ' 'fun!' == 'Dart is fun!' You can dynamically create strings with varying content using `${expression}` to put the string representation of `expression` into a string. string = 'dartlang'; '$string has ${string.length} letters'; // 'dartlang has 8 letters' The value of the expression is automatically converted to a String by callings its `toString` method. You can also concatenate strings (and only strings) using the '+' operator: string = 'dartlang'; string + 'has ' + string.length.toString() + 'letters'; There is no automatic conversion, and the operands need to both be strings. https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart#n... sdk/lib/core/string.dart:130: * invalid UTF-16 string: ... but not a valid Dart String literal: "\uD800" is still a compile time error. https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart#n... sdk/lib/core/string.dart:239: * In error occurs if start is negative or greater than [length]. But do rewrite to "It is an error if [start] is negative or greater than [length]." https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart#n... sdk/lib/core/string.dart:341: * 'resume'.replaceAll(new RegExp(r'e'), '\u00E9'); // 'résumé' Unless it is to distinguish it from "e\u0301" that we pointed out above? https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart#n... sdk/lib/core/string.dart:362: * The function defined below converts each word in a string to 'pig latin' Better say "simplified" since real pig-latin is based on vowel-sounds, not pure syntax (e.g., "y" being a vowel in some cases). https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart#n... sdk/lib/core/string.dart:378: * boundaries and not at rune boundaries: Useful as example because it is a common case, but it's not really a property of String.split, but of the empty string's behavior as a Pattern. Could it be emphasized that this is an example, not a specification of split? https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart#n... sdk/lib/core/string.dart:389: * string.split('').length; // 2 We could define a RunePattern that would always match all of a surrogate pair, or none of it. Using such a const RunePattern('') as argument would not split the string. I.e., it's ''.allMatches behavior, not String.split behavior we are describing here.
https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart File sdk/lib/core/string.dart (right): https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart#n... sdk/lib/core/string.dart:130: * invalid UTF-16 string: On 2013/09/24 08:17:32, Lasse Reichstein Nielsen wrote: > ... but not a valid Dart String literal: "\uD800" is still a compile time error. > Fwiw: we should change that. I filed http://dartbug.com/13535
https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart File sdk/lib/core/string.dart (right): https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart#n... sdk/lib/core/string.dart:26: * string"""; Wrt. a recent mailing list thread, we should document that the last string literal here does not start with a newline.
https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart File sdk/lib/core/string.dart (right): https://codereview.chromium.org/23480035/diff/9001/sdk/lib/core/string.dart#n... sdk/lib/core/string.dart:26: * string"""; On 2013/09/24 10:31:47, Lasse Reichstein Nielsen wrote: > Wrt. a recent mailing list thread, we should document that the last string > literal here does not start with a newline. See my comment at line 12.
https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.dart File sdk/lib/core/string.dart (right): https://chromiumcodereview.appspot.com/23480035/diff/1/sdk/lib/core/string.da... sdk/lib/core/string.dart:8: * A class for working with a sequence of characters. What mem sed. https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... File sdk/lib/core/string.dart (right): https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:118: * You should avoid creating a String with half of a surrogate pair. I'd like to avoid the "It is". Also, why would you want to do this? It seems like something you'd accidentally do, instead of intentionally. How about something like: Creating a String...pair is legal but generally discouraged. On 2013/09/23 15:12:26, floitsch wrote: > That doesn't feel like what I wanted to say. > Let's just write: "It is legal to create a String with only half of a surrogate > pair". (and I'm willing to negotiate on the "only"). https://chromiumcodereview.appspot.com/23480035/diff/9001/sdk/lib/core/string... sdk/lib/core/string.dart:404: * Splits the string based on [pattern], converts both the matched and This is a pretty long description. How about just: Splits the string, converts its parts, and combines them into a new string.
Message was sent while issue was closed.
Committed patchset #3 manually as r27984 (presubmit successful).
Message was sent while issue was closed.
DBC, if a bit late. https://codereview.chromium.org/23480035/diff/20001/sdk/lib/core/string.dart File sdk/lib/core/string.dart (right): https://codereview.chromium.org/23480035/diff/20001/sdk/lib/core/string.dart#... sdk/lib/core/string.dart:10: * A string can be either single or multiline. Single line strings are Wearing my pedant hat: A string itself if neither. A string literal is a way of writing a string value. A string literal can be either .... https://codereview.chromium.org/23480035/diff/20001/sdk/lib/core/string.dart#... sdk/lib/core/string.dart:29: * an operation on a string and assign the result to a new string: You can't perform an operation 'on' a string because it is immutable. Maybe 'with' is more accurate. You can't assign the result to a new string. The new string is a value that is the result of an operation, not something that is assigned to. "Although you cannot change a string, you can perform an operation with a string to produce a new string an assign the result to a variable:" https://codereview.chromium.org/23480035/diff/20001/sdk/lib/core/string.dart#... sdk/lib/core/string.dart:38: * You can also use adjacent string literals for concatenation: This is not really a concatenation operation, it is more a way of breaking up a string literal or interpolated string into pieces. https://codereview.chromium.org/23480035/diff/20001/sdk/lib/core/string.dart#... sdk/lib/core/string.dart:399: * Splits the string, converts its parts, and combines them into a new Perhaps: Splits the string into parts, ... https://codereview.chromium.org/23480035/diff/20001/sdk/lib/core/string.dart#... sdk/lib/core/string.dart:435: * Converts all characters in this string to lower case. This sounds like the string is modified. |
