Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
116673004:
adding new command-line app tutorial (Closed)
|
Created:
7 years ago by mem Modified:
6 years, 11 months ago Base URL:
https://github.com/dart-lang/dartlang.org.git@master Visibility:
Public. |
Descriptionadding new command-line app tutorial
BUG=
R=sethladd@google.com
Committed: 913d4e5
Patch Set 1 #
Total comments: 140
Patch Set 2 : mods based on Kathy's feedback #
Total comments: 56
Patch Set 3 : changes based on comments from Seth #
Total comments: 2
Patch Set 4 : fixed one final tweak from seth #
Messages
Total messages: 9 (0 generated)
Please review the new command-line for big and small. Hosted here: https://tute-cmdline-dot-dart-lang.appspot.com/docs/tutorials/cmdline/ Thanks. mem
Adding seth as a reviewer
In general, looks good. A bunch of nitty stuff, plus some suggestions for organizing/framing the info. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... File src/site/docs/tutorials/cmdline/index.markdown (right): https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:44: <h3>An introduction to command-line apps</h3> change "command-line apps" to something else. (not sure what, but it's repetitive as-is) https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:81: To run a command-line app, you need the Dart VM, VM, -> VM (`dart`), https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:82: which comes with the Dart Editor or the Dart SDK download. -> which comes with the Dart Editor (in the Dart download) or in the [Dart SDK](/tools/sdk/) download. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:84: suppose you installed the Dart download in a directory called `~/myDartDownload`, For example, suppose you installed -> If you install (shave off a few words) https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:85: you can find the Dart VM, called `dart`, the Dart VM, called `dart`, -> `dart` https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:106: int fortyTwo = 'Hello, World'; // Type mis-match mis-match -> mismatch (GLOBAL) https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:143: Checked mode is useful during production for finding errors production -> development https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:158: available to command-line apps, which this tutorial explains. delete ", which this..." Maybe tell people to hover the mouse over green-background text to see explanations? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:205: <a href="#" class="dart-popover" data-toggle="popover" title="Test a path" data-html="true" data-trigger="hover focus" data-content="You can get information about the file system on which your program is running.">FileSystemEntity.isDirectory(path)</a>.then((isDir) {if (isDir) { if (isDir) { should be on its own line, right? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:237: Let's take a look at the `dcat` sample sample -> sample, https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:247: Run the program from the command-line as shown by the highlighted text. command-line -> command line (since it's not an adjective here) https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:249: {% prettify bash %} Weirdly, this highlights seemingly random words, such as Dart and All. Not sure what a better formatter would be. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:250: $ [[highlight]]dart dcat.dart -n dcat.dart[[/highlight]] I found the -n dcat.dart confusing, since dcat.dart was used twice, and I didn't know what was an argument to what. Using a second file (maybe a .txt file) would make it clearer. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:279: void main(<a href="#" class="dart-popover" data-toggle="popover" title="Command-line arguments" data-html="true" data-trigger="hover focus" data-content="The system passes command line arguments into the program in a list of strings.">List<String> arguments</a>) { command line -> command-line :) https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:292: <a href="https://api.dartlang.org/args.html" target="_blank">args</a> library This link should be on API docs (and why isn't it markdown?). Maybe: The [API docs for the args library](...) https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:293: provides very detailed information provides very -> provide https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:294: about the use of the ArgsParser and ArgResults classes. about the use of -> to help you use OR about using https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:300: The standard I/O streams are defined at the top-level of the dart:io library, top-level -> top level https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:319: | Library | Description | Library -> Object? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:340: convert it to a string, and print it. The `writelin()` method lin -> ln https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:355: and can be redirected or piped at the command-line to different destinations. no - in command-line here! https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:383: stdout.writeln('What's in a name?'); 's => \'s? (This doesn't work as written, right?) (It'll still look bad in the formatter, though. Maybe use " instead of '.) https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:392: The `readLineSync()` method reads text from the standard input stream stream -> stream, https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:414: [[highlight]]ctl-d[[/highlight]] ctl-d -> <ctl-d> ? (so it'll match the description in the text) https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:425: you can determine whether the path is a file, a directory, a link, or not found. It's a little weird that we've already seen this. Maybe do a forward link from there? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:427: you can use the `type()` method from the `FileSystemEntity` class. This is linked to the previous example, but not to the preceding sentence... it doesn't say what `type()` does. Merge them? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:428: Because the `type` method accesses the file system, type or type()? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:433: on the command-line is a directory. command-line -> command line https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:434: The Future returns a boolean to the `then()` method that indicates It's not actually returned to the then() method. It's an argument to the handler that then registers. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:449: include `isFile()`, `exists()`, `stat()` and `parent`, and -> , and https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:452: and renaming a file. Do they not return Futures? Or are there multiple deletion/rename methods? Having these separate from the methods in the previous sentence (and not naming the methods) made me wonder... https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:456: For each file listed on the command-line `dcat` opens it for reading This sentence needs a little work, grammatically. (and command-line -> command line!) Maybe split it into multiple sentences? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:458: This returns a stream. Seems awkward. Combine with previous sentence? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:462: The callback function provided by `dcat` writes the data to stdout. This is a little disjointed. It's unclear why it's a separate paragraph, and it's unclear what I'm supposed to look for in the following code example. E.g. I was surprised to see listen() highlighted below, since I expected the callback function to be the important thing. Maybe this should all be one paragraph? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:476: }).asFuture().then((_) { exitCode = 1; }).catchError((_) => _handleError(path)); reduce the line length here https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:498: }).asFuture().then((_) { exitCode = 1; }).catchError((_) => _handleError(path)); another too-long line https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:501: including -> , including https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:515: File remove newline after File https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:521: String stronger = 'That which does not kill us makes us stronger. -Friedrich Nietzsche'; long line https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:529: To append data to an existing file you can use the optional long sentence, no commas! https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:532: if there is any, gets overwritten. delete "there is" gets -> are https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:536: You can continue to write to the file until done. Combine this with the following sentence, to make it less choppy. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:553: Platform remove newline after Platform https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:555: class (from the dart:io library, not from the dart:html library) make this io vs. html aside a note, so it doesn't interrupt the flow https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:558: The `environmont` property returns a Map that contains all environmOnt? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:574: in the Map, but these do not persist and if you refer to , but -> However, persist and -> persist. If (You can't see from the code that they don't persist.) https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:578: information about the machine and OS: Are the following all of the properties? If so, reign back expectations (I see nothing about machines here). If not, say this is an example (or otherwise make clear that this is a subset). https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:582: Platform.isLinux() usually we don't use a <pre> section for non-running code. maybe use a list instead? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:588: {% prettify dart %} This seems a little weird; it's not code you'd use. Maybe just fold this <pre> into the previous sentence? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:594: The dart:io library defines a top-level property property -> property, https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:606: exitCode = 0; //presume success. hmmm... I just noticed that comment format is different from our usual. I expected: // Presume success. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:636: For example, the `_handleError` function could use this code _handleError -> _handleError() https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:640: _handleError(String path) { I might just put this inline, since you don't encourage using exit(). Maybe For example, the `_handleError()` function could call `exit(2)` instead of setting `exitCode` to 2. Then delete this code section. (Remember, people blindly copy code!) https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:657: these are defined by convention: I don't like "these" (or "are defined"). Rewrite if you can. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:666: ## Check out the code for another example {#dgrep-code} Maybe just: ## Another example: dgrep {#dgrep-code} https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:668: Here's the code for a simplified version of the Unix utility grep for a... -> for `dgrep`, a .... https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:670: This program makes use of some additional APIs that do not do not -> don't https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:673: <pre class="prettyprint lang-dart"> why use a <pre> here instead of {% prettify... %}? Is that necessary for highlighting? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:702: final startingDir = <a href="#" class="dart-popover" data-toggle="popover" title="Create a Directory" data-html="true" data-trigger="hover focus" data-content="Create a new Directory object with a pathname.">new Directory(searchPath);</a> What does "with a pathname" mean? How does it differ from "with a path" below? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:710: searchFile(<a href="#" class="dart-popover" data-toggle="popover" title="Create a File" data-html="true" data-trigger="hover focus" data-content="Create a new file with a path">new File(searchPath)</a>, searchTerms); path -> path. maybe: with a path -> at the specified path? (I wasn't sure what "with" meant.) https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:742: One kind of command-line app that you can write is an HTTP server. I'd lead with the link, instead of the discussion. Maybe: See the [Dartiverse Search walkthrough](/docs/dart-up-and-running/contents/ch05.html) for an example of another kind of command-line app: an HTTP server. (That reminds me... I wish we had a good way to link to each sample. If each sample had a homepage that pointed to all relevant pages, that'd be cool.) https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:747: This tutorial described some basic API found in these classes from the dart:io library: This intro makes this sound like a summary of the page, rather than links to other resources. Actually... it'd make sense to put this in a new "Summary" section, since much of this info (and the links) is elsewhere, and this is just a handy, one-stop place to find them. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:749: | Class | Description | Class -> API? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:763: In addition, this tutorial covers two classes that help with command-line arguments: This, too, is more "Summary" than "Other resources" https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:771: Refer to the API docs for <a href="https://api.dartlang.org/dart_io.html" target="_blank">dart:io</a>, This seems like genuine "Other resources" material. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:781: in which the server is an HttpServer. Maybe delete this line, ending the sentence at "example." It doesn't seem necessary to talk about HttpServer here, does it? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... File src/site/docs/tutorials/index.markdown (right): https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/index.markdown:4: description: "The Dart Tutorials—Your guide to building great web apps." Perhaps we need to revise this tagline, now that we have command-line apps, as well? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/index.markdown:19: **The Dart Tutorials** teach you how to build web applications web applications -> applications? Maybe add: The focus is on web apps, but you can also find information on writing command-line apps. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/index.markdown:40: <li><a href="#futures" data-toggle="tab">Futures & Streams</a></li> Why have Streams here? I see no Sream tutorial... If you do keep it, Streams -> streams? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/index.markdown:41: <li><a href="#forms" data-toggle="tab">Forms & Data</a></li> Data -> data (or maybe make all the tabs Title Case, since it's a little weird to have "Get started" and "Get Started" on the same page.) https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/index.markdown:253: <h4 class="no-permalink"><a href="cmdline/"><img src="images/target.png" height="20" width="20"> Write Command-line Apps</a></h4> -line -> -Line? https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/index.markdown:255: <img src="cmdline/images/commandcode.png" width="350"> This image is broken for me
PTAL. mem https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... File src/site/docs/tutorials/cmdline/index.markdown (right): https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:44: <h3>An introduction to command-line apps</h3> On 2013/12/20 23:12:16, Kathy Walrath wrote: > change "command-line apps" to something else. (not sure what, but it's > repetitive as-is) Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:81: To run a command-line app, you need the Dart VM, On 2013/12/20 23:12:16, Kathy Walrath wrote: > VM, -> VM (`dart`), Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:82: which comes with the Dart Editor or the Dart SDK download. On 2013/12/20 23:12:16, Kathy Walrath wrote: > -> which comes with the Dart Editor (in the Dart download) or in the > [Dart SDK](/tools/sdk/) download. Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:84: suppose you installed the Dart download in a directory called `~/myDartDownload`, On 2013/12/20 23:12:16, Kathy Walrath wrote: > For example, suppose you installed -> If you install > > (shave off a few words) Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:85: you can find the Dart VM, called `dart`, On 2013/12/20 23:12:16, Kathy Walrath wrote: > the Dart VM, called `dart`, -> `dart` Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:106: int fortyTwo = 'Hello, World'; // Type mis-match On 2013/12/20 23:12:16, Kathy Walrath wrote: > mis-match -> mismatch > > (GLOBAL) Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:143: Checked mode is useful during production for finding errors On 2013/12/20 23:12:16, Kathy Walrath wrote: > production -> development Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:158: available to command-line apps, which this tutorial explains. On 2013/12/20 23:12:16, Kathy Walrath wrote: > delete ", which this..." > > Maybe tell people to hover the mouse over green-background text to see > explanations? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:205: <a href="#" class="dart-popover" data-toggle="popover" title="Test a path" data-html="true" data-trigger="hover focus" data-content="You can get information about the file system on which your program is running.">FileSystemEntity.isDirectory(path)</a>.then((isDir) {if (isDir) { On 2013/12/20 23:12:16, Kathy Walrath wrote: > if (isDir) { > should be on its own line, right? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:237: Let's take a look at the `dcat` sample On 2013/12/20 23:12:16, Kathy Walrath wrote: > sample -> sample, Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:247: Run the program from the command-line as shown by the highlighted text. On 2013/12/20 23:12:16, Kathy Walrath wrote: > command-line -> command line > > (since it's not an adjective here) Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:250: $ [[highlight]]dart dcat.dart -n dcat.dart[[/highlight]] On 2013/12/20 23:12:16, Kathy Walrath wrote: > I found the -n dcat.dart confusing, since dcat.dart was used twice, and I didn't > know what was an argument to what. > > Using a second file (maybe a .txt file) would make it clearer. Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:279: void main(<a href="#" class="dart-popover" data-toggle="popover" title="Command-line arguments" data-html="true" data-trigger="hover focus" data-content="The system passes command line arguments into the program in a list of strings.">List<String> arguments</a>) { On 2013/12/20 23:12:16, Kathy Walrath wrote: > command line -> command-line > > :) Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:292: <a href="https://api.dartlang.org/args.html" target="_blank">args</a> library On 2013/12/20 23:12:16, Kathy Walrath wrote: > This link should be on API docs (and why isn't it markdown?). Maybe: > > The [API docs for the args library](...) it's not in markdown because I want to use target=_blank. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:293: provides very detailed information On 2013/12/20 23:12:16, Kathy Walrath wrote: > provides very -> provide Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:294: about the use of the ArgsParser and ArgResults classes. On 2013/12/20 23:12:16, Kathy Walrath wrote: > about the use of > -> > to help you use > OR > about using Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:300: The standard I/O streams are defined at the top-level of the dart:io library, On 2013/12/20 23:12:16, Kathy Walrath wrote: > top-level -> top level Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:319: | Library | Description | On 2013/12/20 23:12:16, Kathy Walrath wrote: > Library -> Object? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:340: convert it to a string, and print it. The `writelin()` method On 2013/12/20 23:12:16, Kathy Walrath wrote: > lin -> ln Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:340: convert it to a string, and print it. The `writelin()` method On 2013/12/20 23:12:16, Kathy Walrath wrote: > lin -> ln Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:355: and can be redirected or piped at the command-line to different destinations. On 2013/12/20 23:12:16, Kathy Walrath wrote: > no - in command-line here! Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:383: stdout.writeln('What's in a name?'); On 2013/12/20 23:12:16, Kathy Walrath wrote: > 's => \'s? (This doesn't work as written, right?) > > (It'll still look bad in the formatter, though. Maybe use " instead of '.) Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:392: The `readLineSync()` method reads text from the standard input stream On 2013/12/20 23:12:16, Kathy Walrath wrote: > stream -> stream, Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:414: [[highlight]]ctl-d[[/highlight]] On 2013/12/20 23:12:16, Kathy Walrath wrote: > ctl-d -> <ctl-d> > ? > > (so it'll match the description in the text) the < and gt& signs don't work with highlight. So I used ... Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:425: you can determine whether the path is a file, a directory, a link, or not found. On 2013/12/20 23:12:16, Kathy Walrath wrote: > It's a little weird that we've already seen this. > Maybe do a forward link from there? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:427: you can use the `type()` method from the `FileSystemEntity` class. On 2013/12/20 23:12:16, Kathy Walrath wrote: > This is linked to the previous example, but not to the preceding sentence... it > doesn't say what `type()` does. Merge them? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:428: Because the `type` method accesses the file system, On 2013/12/20 23:12:16, Kathy Walrath wrote: > type or type()? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:433: on the command-line is a directory. On 2013/12/20 23:12:16, Kathy Walrath wrote: > command-line -> command line Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:434: The Future returns a boolean to the `then()` method that indicates On 2013/12/20 23:12:16, Kathy Walrath wrote: > It's not actually returned to the then() method. It's an argument to the handler > that then registers. Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:449: include `isFile()`, `exists()`, `stat()` and `parent`, On 2013/12/20 23:12:16, Kathy Walrath wrote: > and -> , and Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:452: and renaming a file. On 2013/12/20 23:12:16, Kathy Walrath wrote: > Do they not return Futures? Or are there multiple deletion/rename methods? > > Having these separate from the methods in the previous sentence (and not naming > the methods) made me wonder... Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:456: For each file listed on the command-line `dcat` opens it for reading On 2013/12/20 23:12:16, Kathy Walrath wrote: > This sentence needs a little work, grammatically. (and command-line -> command > line!) > > Maybe split it into multiple sentences? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:456: For each file listed on the command-line `dcat` opens it for reading On 2013/12/20 23:12:16, Kathy Walrath wrote: > This sentence needs a little work, grammatically. (and command-line -> command > line!) > > Maybe split it into multiple sentences? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:458: This returns a stream. On 2013/12/20 23:12:16, Kathy Walrath wrote: > Seems awkward. Combine with previous sentence? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:462: The callback function provided by `dcat` writes the data to stdout. On 2013/12/20 23:12:16, Kathy Walrath wrote: > This is a little disjointed. It's unclear why it's a separate paragraph, and > it's unclear what I'm supposed to look for in the following code example. E.g. I > was surprised to see listen() highlighted below, since I expected the callback > function to be the important thing. > > Maybe this should all be one paragraph? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:476: }).asFuture().then((_) { exitCode = 1; }).catchError((_) => _handleError(path)); On 2013/12/20 23:12:16, Kathy Walrath wrote: > reduce the line length here Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:498: }).asFuture().then((_) { exitCode = 1; }).catchError((_) => _handleError(path)); On 2013/12/20 23:12:16, Kathy Walrath wrote: > another too-long line Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:501: On 2013/12/20 23:12:16, Kathy Walrath wrote: > including -> , including Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:515: File On 2013/12/20 23:12:16, Kathy Walrath wrote: > remove newline after File Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:521: String stronger = 'That which does not kill us makes us stronger. -Friedrich Nietzsche'; On 2013/12/20 23:12:16, Kathy Walrath wrote: > long line Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:529: To append data to an existing file you can use the optional On 2013/12/20 23:12:16, Kathy Walrath wrote: > long sentence, no commas! Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:532: if there is any, gets overwritten. On 2013/12/20 23:12:16, Kathy Walrath wrote: > delete "there is" > gets -> are Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:536: You can continue to write to the file until done. On 2013/12/20 23:12:16, Kathy Walrath wrote: > Combine this with the following sentence, to make it less choppy. Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:553: Platform On 2013/12/20 23:12:16, Kathy Walrath wrote: > remove newline after Platform Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:555: class (from the dart:io library, not from the dart:html library) On 2013/12/20 23:12:16, Kathy Walrath wrote: > make this io vs. html aside a note, so it doesn't interrupt the flow Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:555: class (from the dart:io library, not from the dart:html library) On 2013/12/20 23:12:16, Kathy Walrath wrote: > make this io vs. html aside a note, so it doesn't interrupt the flow Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:558: The `environmont` property returns a Map that contains all On 2013/12/20 23:12:16, Kathy Walrath wrote: > environmOnt? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:574: in the Map, but these do not persist and if you refer to On 2013/12/20 23:12:16, Kathy Walrath wrote: > , but -> However, > persist and -> persist. If > > (You can't see from the code that they don't persist.) Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:578: information about the machine and OS: On 2013/12/20 23:12:16, Kathy Walrath wrote: > Are the following all of the properties? If so, reign back expectations (I see > nothing about machines here). If not, say this is an example (or otherwise make > clear that this is a subset). Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:582: Platform.isLinux() On 2013/12/20 23:12:16, Kathy Walrath wrote: > usually we don't use a <pre> section for non-running code. maybe use a list > instead? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:588: {% prettify dart %} On 2013/12/20 23:12:16, Kathy Walrath wrote: > This seems a little weird; it's not code you'd use. Maybe just fold this <pre> > into the previous sentence? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:594: The dart:io library defines a top-level property On 2013/12/20 23:12:16, Kathy Walrath wrote: > property -> property, Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:606: exitCode = 0; //presume success. On 2013/12/20 23:12:16, Kathy Walrath wrote: > hmmm... I just noticed that comment format is different from our usual. I > expected: > > // Presume success. Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:636: For example, the `_handleError` function could use this code On 2013/12/20 23:12:16, Kathy Walrath wrote: > _handleError -> _handleError() Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:636: For example, the `_handleError` function could use this code On 2013/12/20 23:12:16, Kathy Walrath wrote: > _handleError -> _handleError() Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:640: _handleError(String path) { On 2013/12/20 23:12:16, Kathy Walrath wrote: > I might just put this inline, since you don't encourage using exit(). Maybe > > For example, the `_handleError()` function could call `exit(2)` instead of > setting `exitCode` to 2. > > Then delete this code section. (Remember, people blindly copy code!) Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:657: these are defined by convention: On 2013/12/20 23:12:16, Kathy Walrath wrote: > I don't like "these" (or "are defined"). Rewrite if you can. Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:666: ## Check out the code for another example {#dgrep-code} On 2013/12/20 23:12:16, Kathy Walrath wrote: > Maybe just: > > ## Another example: dgrep {#dgrep-code} Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:668: Here's the code for a simplified version of the Unix utility grep On 2013/12/20 23:12:16, Kathy Walrath wrote: > for a... -> for `dgrep`, a .... Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:670: This program makes use of some additional APIs that do not On 2013/12/20 23:12:16, Kathy Walrath wrote: > do not -> don't Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:673: <pre class="prettyprint lang-dart"> On 2013/12/20 23:12:16, Kathy Walrath wrote: > why use a <pre> here instead of {% prettify... %}? > > Is that necessary for highlighting? it's necessary for the popups. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:702: final startingDir = <a href="#" class="dart-popover" data-toggle="popover" title="Create a Directory" data-html="true" data-trigger="hover focus" data-content="Create a new Directory object with a pathname.">new Directory(searchPath);</a> On 2013/12/20 23:12:16, Kathy Walrath wrote: > What does "with a pathname" mean? How does it differ from "with a path" below? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:710: searchFile(<a href="#" class="dart-popover" data-toggle="popover" title="Create a File" data-html="true" data-trigger="hover focus" data-content="Create a new file with a path">new File(searchPath)</a>, searchTerms); On 2013/12/20 23:12:16, Kathy Walrath wrote: > path -> path. > > maybe: with a path -> at the specified path? > > (I wasn't sure what "with" meant.) Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:747: This tutorial described some basic API found in these classes from the dart:io library: On 2013/12/20 23:12:16, Kathy Walrath wrote: > This intro makes this sound like a summary of the page, rather than links to > other resources. > > Actually... it'd make sense to put this in a new "Summary" section, since much > of this info (and the links) is elsewhere, and this is just a handy, one-stop > place to find them. Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:749: | Class | Description | On 2013/12/20 23:12:16, Kathy Walrath wrote: > Class -> API? Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:763: In addition, this tutorial covers two classes that help with command-line arguments: On 2013/12/20 23:12:16, Kathy Walrath wrote: > This, too, is more "Summary" than "Other resources" Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:771: Refer to the API docs for <a href="https://api.dartlang.org/dart_io.html" target="_blank">dart:io</a>, On 2013/12/20 23:12:16, Kathy Walrath wrote: > This seems like genuine "Other resources" material. Done. https://chromiumcodereview.appspot.com/116673004/diff/1/src/site/docs/tutoria... src/site/docs/tutorials/cmdline/index.markdown:781: in which the server is an HttpServer. On 2013/12/20 23:12:16, Kathy Walrath wrote: > Maybe delete this line, ending the sentence at "example." It doesn't seem > necessary to talk about HttpServer here, does it? Done.
https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... File src/site/docs/tutorials/cmdline/index.markdown (right): https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:14: {% capture whats_the_point %} Global: I'd like to see some advice for when to use the sync versions and when to use the async (future, stream) versions of the APIs. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:14: {% capture whats_the_point %} Global: I couldn't find it, can you link to the programmer's guide from this doc? https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:16: * Server-side applications need to do input and output. the tutorial says "command-line" but here you say server-side https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:33: Don't have the source code? the source for the examples above? https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:44: <h3>An introduction to standalone apps</h3> and here we use standalone (instead of command-line or server-side) https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:61: This tutorial looks at a few small command-line applications. opinion: this tutorial teaches you how to build command-line applications. "looks at" seems like I'm not involved. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:80: ## Run an app with the standalone Dart VM {#run-the-first-app} other headers are "-ing" like "reading" or "writing". Should this be "running".. ? https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:83: which comes with the Dart Editor (in the Dart download) use a link for "Dart download" ? https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:107: int fortyTwo = 'Hello, World'; // Type mismatch. I think teaching the optional types here, and how to run a dart app, are too much. Experience shows that if we try to introduce this kind of behavior too soon, it's a turn-off. Can we show a perfectly working example first, and later tackle --checked ? I was expecting the use of var here, because it's easy for type inference to kick in when you initialize the variable. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:154: ## Check out the dcat example {#dcat-code} Can you make it more clear that you'll go into more detail after this look at the whole program? I was confused and thought I needed to digest all of this now. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:157: which is a simplified implementation of the Unix cat utility. explain what the Unix cat utility is. 80% of our users are on windows, they might now know what this is. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:175: exitCode = 0; // Presume success. I'm left wondering where exitCode came from? https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:223: package provides Are they going to know what a package is? https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:316: rather than streams, but can be bound to a stream. what is a stream? https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:330: ### stdout maybe add an aside about print(), and when to use print() vs stdout.write https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:373: ### stdin I think it would be nice to bring these sub-sections back to dcat in a more obvious way. Explain stdin, but then say "dcat uses stdin to __ and __" This applies to "getting info about a file", etc https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:575: in the Map. However, these do not persist. If you refer to persist is a bit overloaded. I think this is a bug in the API, and/or there's another way to explain this. "You get a copy of the env variables in a mutable map." https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:590: `exitCode`, that you can change to set the exit code for define what an exit code is https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:601: exitCode = 0; // Presume success. weird that it sets it to zero at the beginning. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:631: For example, the `_handleError()` function could call `exit(2) missing a closing ` https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:633: Generally speaking, you are better off using the `exitCode` property, make this bolder, more obvious. Do we have a "Mary's Tips!" way to callout insight? https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:647: ### Another example: dgrep {#dgrep-code} I think this is the wrong header level? Too low. Also, what is dgrep? Can you use a title that's more descriptive? https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:647: ### Another example: dgrep {#dgrep-code} Consider dropping this section. It doesn't provide enough value. What new concepts are being introduced? https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:649: Here's the code for `dgrep`, a simplified version of the Unix utility grep Again, what is grep? https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:651: This program makes use of some additional APIs that don't instead of "additional APIs", how about enumerating what else it's goign to show me? Help me decide if I want to keep reading... https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:678: final searchPath = <a href="#" class="dart-popover" data-toggle="popover" title="The rest property is a List." data-html="true" data-trigger="hover focus" data-content="ArgResults.rest is a list. Use the last property to get the last command-line argument.">argResults.rest.last;</a> not that interesting, consider dropping (it's not specific to I/O) https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:691: searchFile(<a href="#" class="dart-popover" data-toggle="popover" title="Create a File" data-html="true" data-trigger="hover focus" data-content="Create a new file at the specified path.">new File(searchPath)</a>, searchTerms); This doesn't actually create a new file. I don't think. I think it gets created when you write to it. "Create" is overloaded.
PTAL https://tute-cmdline-dot-dart-lang.appspot.com/docs/tutorials/cmdline https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... File src/site/docs/tutorials/cmdline/index.markdown (right): https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:14: {% capture whats_the_point %} On 2014/01/03 00:57:28, sethladd wrote: > Global: I couldn't find it, can you link to the programmer's guide from this > doc? Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:14: {% capture whats_the_point %} On 2014/01/03 00:57:28, sethladd wrote: > Global: I'd like to see some advice for when to use the sync versions and when > to use the async (future, stream) versions of the APIs. There's only one place where synchronous is used and it's with stdin and I do talk about it. All other uses are as customers of a Future where you don't have a choice https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:16: * Server-side applications need to do input and output. On 2014/01/03 00:57:28, sethladd wrote: > the tutorial says "command-line" but here you say server-side Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:33: Don't have the source code? this is consistent with all of the other tutorials. On 2014/01/03 00:57:28, sethladd wrote: > the source for the examples above? https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:44: <h3>An introduction to standalone apps</h3> On 2014/01/03 00:57:28, sethladd wrote: > and here we use standalone (instead of command-line or server-side) we use standalone here otherwise we use the exact same words in the page title as in the sub-title. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:61: This tutorial looks at a few small command-line applications. On 2014/01/03 00:57:28, sethladd wrote: > opinion: this tutorial teaches you how to build command-line applications. > "looks at" seems like I'm not involved. Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:80: ## Run an app with the standalone Dart VM {#run-the-first-app} On 2014/01/03 00:57:28, sethladd wrote: > other headers are "-ing" like "reading" or "writing". Should this be "running".. > ? Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:83: which comes with the Dart Editor (in the Dart download) On 2014/01/03 00:57:28, sethladd wrote: > use a link for "Dart download" ? Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:107: int fortyTwo = 'Hello, World'; // Type mismatch. On 2014/01/03 00:57:28, sethladd wrote: > I think teaching the optional types here, and how to run a dart app, are too > much. Experience shows that if we try to introduce this kind of behavior too > soon, it's a turn-off. Can we show a perfectly working example first, and later > tackle --checked ? > > I was expecting the use of var here, because it's easy for type inference to > kick in when you initialize the variable. Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:154: ## Check out the dcat example {#dcat-code} On 2014/01/03 00:57:28, sethladd wrote: > Can you make it more clear that you'll go into more detail after this look at > the whole program? I was confused and thought I needed to digest all of this > now. Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:157: which is a simplified implementation of the Unix cat utility. On 2014/01/03 00:57:28, sethladd wrote: > explain what the Unix cat utility is. 80% of our users are on windows, they > might now know what this is. Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:175: exitCode = 0; // Presume success. On 2014/01/03 00:57:28, sethladd wrote: > I'm left wondering where exitCode came from? Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:223: package provides On 2014/01/03 00:57:28, sethladd wrote: > Are they going to know what a package is? Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:316: rather than streams, but can be bound to a stream. On 2014/01/03 00:57:28, sethladd wrote: > what is a stream? Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:330: ### stdout On 2014/01/03 00:57:28, sethladd wrote: > maybe add an aside about print(), and when to use print() vs stdout.write Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:373: ### stdin On 2014/01/03 00:57:28, sethladd wrote: > I think it would be nice to bring these sub-sections back to dcat in a more > obvious way. Explain stdin, but then say "dcat uses stdin to __ and __" > > This applies to "getting info about a file", etc Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:575: in the Map. However, these do not persist. If you refer to On 2014/01/03 00:57:28, sethladd wrote: > persist is a bit overloaded. I think this is a bug in the API, and/or there's > another way to explain this. "You get a copy of the env variables in a mutable > map." Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:590: `exitCode`, that you can change to set the exit code for On 2014/01/03 00:57:28, sethladd wrote: > define what an exit code is Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:601: exitCode = 0; // Presume success. On 2014/01/03 00:57:28, sethladd wrote: > weird that it sets it to zero at the beginning. Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:631: For example, the `_handleError()` function could call `exit(2) On 2014/01/03 00:57:28, sethladd wrote: > missing a closing ` Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:633: Generally speaking, you are better off using the `exitCode` property, On 2014/01/03 00:57:28, sethladd wrote: > make this bolder, more obvious. Do we have a "Mary's Tips!" way to callout > insight? Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:647: ### Another example: dgrep {#dgrep-code} On 2014/01/03 00:57:28, sethladd wrote: > I think this is the wrong header level? Too low. Also, what is dgrep? Can you > use a title that's more descriptive? Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:647: ### Another example: dgrep {#dgrep-code} On 2014/01/03 00:57:28, sethladd wrote: > I think this is the wrong header level? Too low. Also, what is dgrep? Can you > use a title that's more descriptive? Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:647: ### Another example: dgrep {#dgrep-code} On 2014/01/03 00:57:28, sethladd wrote: > Consider dropping this section. It doesn't provide enough value. What new > concepts are being introduced? Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:649: Here's the code for `dgrep`, a simplified version of the Unix utility grep On 2014/01/03 00:57:28, sethladd wrote: > Again, what is grep? Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:649: Here's the code for `dgrep`, a simplified version of the Unix utility grep On 2014/01/03 00:57:28, sethladd wrote: > Again, what is grep? Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:651: This program makes use of some additional APIs that don't On 2014/01/03 00:57:28, sethladd wrote: > instead of "additional APIs", how about enumerating what else it's goign to show > me? Help me decide if I want to keep reading... Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:678: final searchPath = <a href="#" class="dart-popover" data-toggle="popover" title="The rest property is a List." data-html="true" data-trigger="hover focus" data-content="ArgResults.rest is a list. Use the last property to get the last command-line argument.">argResults.rest.last;</a> On 2014/01/03 00:57:28, sethladd wrote: > not that interesting, consider dropping (it's not specific to I/O) Done. https://chromiumcodereview.appspot.com/116673004/diff/60001/src/site/docs/tut... src/site/docs/tutorials/cmdline/index.markdown:691: searchFile(<a href="#" class="dart-popover" data-toggle="popover" title="Create a File" data-html="true" data-trigger="hover focus" data-content="Create a new file at the specified path.">new File(searchPath)</a>, searchTerms); On 2014/01/03 00:57:28, sethladd wrote: > This doesn't actually create a new file. I don't think. I think it gets created > when you write to it. > > "Create" is overloaded. Done.
lgtm with an open note about --checked https://chromiumcodereview.appspot.com/116673004/diff/150001/src/site/docs/tu... File src/site/docs/tutorials/cmdline/index.markdown (right): https://chromiumcodereview.appspot.com/116673004/diff/150001/src/site/docs/tu... src/site/docs/tutorials/cmdline/index.markdown:113: You can run the Dart VM in checked mode by using the `--checked` flag. Can we lead with why you might want to do this? Or, we can punt on this and just say "there's lots of options for dart, go here to learn more about them"
Message was sent while issue was closed.
Committed patchset #4 manually as r913d4e5 (presubmit successful).
Message was sent while issue was closed.
THanks for your comments. All done. https://chromiumcodereview.appspot.com/116673004/diff/150001/src/site/docs/tu... File src/site/docs/tutorials/cmdline/index.markdown (right): https://chromiumcodereview.appspot.com/116673004/diff/150001/src/site/docs/tu... src/site/docs/tutorials/cmdline/index.markdown:113: You can run the Dart VM in checked mode by using the `--checked` flag. On 2014/01/03 17:54:54, sethladd wrote: > Can we lead with why you might want to do this? Or, we can punt on this and just > say "there's lots of options for dart, go here to learn more about them" Done. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
