Wrap comments at 80 characters (#593)

I think 70 is too agressive, especially since it includes indentation
when determining line length.

rustfmt.toml

@@ -1,4 +1,4 @@
-comment_width                = 70
+comment_width                = 80
 edition                      = "2018"
 error_on_line_overflow       = true
 error_on_unformatted         = true
@@ -13,7 +13,7 @@ newline_style                = "Unix"
 normalize_comments           = true
 overflow_delimited_expr      = true
 reorder_impl_items           = true
-required_version             = "1.4.11"
+required_version             = "1.4.12"
 struct_field_align_threshold = 20
 tab_spaces                   = 2
 unstable_features            = true

src/config.rs

@@ -552,10 +552,9 @@ impl Config {
           }
         }
 
-        // Declaring this outside of the nested loops will probably be more
+        // Declaring this outside of the nested loops will probably be more efficient,
-        // efficient, but it creates all sorts of lifetime issues with
+        // but it creates all sorts of lifetime issues with variables inside the loops.
-        // variables inside the loops. If this is inlined like the
+        // If this is inlined like the docs say, it shouldn't make any difference.
-        // docs say, it shouldn't make any difference.
         let print_doc = |doc| {
           print!(
             " {:padding$}{} {}",
@@ -641,9 +640,8 @@ mod tests {
 
   use pretty_assertions::assert_eq;
 
-  // This test guards against unintended changes to the argument parser.
+  // This test guards against unintended changes to the argument parser. We should
-  // We should have proper tests for all the flags, but this will do
+  // have proper tests for all the flags, but this will do for now.
-  // for now.
   #[test]
   fn help() {
     const EXPECTED_HELP: &str = "just v0.5.8

src/expression.rs

@@ -1,9 +1,9 @@
 use crate::common::*;
 
-/// An expression. Note that the Just language grammar has both an
+/// An expression. Note that the Just language grammar has both an `expression`
-/// `expression` production of additions (`a + b`) and values, and a
+/// production of additions (`a + b`) and values, and a `value` production of
-/// `value` production of all other value types (for example strings,
+/// all other value types (for example strings, function calls, and
-/// function calls, and parenthetical groups).
+/// parenthetical groups).
 ///
 /// The parser parses both values and expressions into `Expression`s.
 #[derive(PartialEq, Debug)]

src/justfile.rs

@@ -301,13 +301,11 @@ mod tests {
     }
   }
 
-  // this test exists to make sure that shebang recipes
+  // This test exists to make sure that shebang recipes run correctly.  Although
-  // run correctly. although this script is still
+  // this script is still executed by a shell its behavior depends on the value of
-  // executed by a shell its behavior depends on the value of a
+  // a variable and continuing even though a command fails, whereas in plain
-  // variable and continuing even though a command fails,
+  // recipes variables are not available in subsequent lines and execution stops
-  // whereas in plain recipes variables are not available
+  // when a line fails.
-  // in subsequent lines and execution stops when a line
-  // fails
   run_error! {
     name: run_shebang,
     src: "

src/lexer.rs

@@ -5,12 +5,12 @@ use TokenKind::*;
 
 /// Just language lexer
 ///
-/// The lexer proceeds character-by-character, as opposed to using
+/// The lexer proceeds character-by-character, as opposed to using regular
-/// regular expressions to lex tokens or semi-tokens at a time. As a
+/// expressions to lex tokens or semi-tokens at a time. As a result, it is
-/// result, it is verbose and straightforward. Just used to have a
+/// verbose and straightforward. Just used to have a regex-based lexer, which
-/// regex-based lexer, which was slower and generally godawful.
+/// was slower and generally godawful.  However, this should not be taken as a
-/// However, this should not be taken as a slight against regular
+/// slight against regular expressions, the lexer was just idiosyncratically
-/// expressions, the lexer was just idiosyncratically bad.
+/// bad.
 pub(crate) struct Lexer<'src> {
   /// Source text
   src:                 &'src str,
@@ -65,8 +65,8 @@ impl<'src> Lexer<'src> {
     }
   }
 
-  /// Advance over the character in `self.next`, updating
+  /// Advance over the character in `self.next`, updating `self.token_end`
-  /// `self.token_end` accordingly.
+  /// accordingly.
   fn advance(&mut self) -> CompilationResult<'src, ()> {
     match self.next {
       Some(c) => {
@@ -138,8 +138,8 @@ impl<'src> Lexer<'src> {
     !self.indentation().is_empty()
   }
 
-  /// Create a new token with `kind` whose lexeme
+  /// Create a new token with `kind` whose lexeme is between `self.token_start`
-  /// is between `self.token_start` and `self.token_end`
+  /// and `self.token_end`
   fn token(&mut self, kind: TokenKind) {
     self.tokens.push(Token {
       offset: self.token_start.offset,
@@ -177,8 +177,7 @@ impl<'src> Lexer<'src> {
   fn error(&self, kind: CompilationErrorKind<'src>) -> CompilationError<'src> {
     // Use the in-progress token span as the location of the error.
 
-    // The width of the error site to highlight depends on the kind of
+    // The width of the error site to highlight depends on the kind of error:
-    // error:
     let length = match kind {
       // highlight ' or "
       UnterminatedString => 1,

src/line.rs

@@ -1,7 +1,6 @@
 use crate::common::*;
 
-/// A single line in a recipe body, consisting of any number of
+/// A single line in a recipe body, consisting of any number of `Fragment`s.
-/// `Fragment`s.
 #[derive(Debug, PartialEq)]
 pub(crate) struct Line<'src> {
   pub(crate) fragments: Vec<Fragment<'src>>,

src/module.rs

@@ -1,13 +1,12 @@
 use crate::common::*;
 
-/// A module, the top-level type produced by the parser. So-named
+/// A module, the top-level type produced by the parser. So-named because
-/// because although at present, all justfiles consist of a single
+/// although at present, all justfiles consist of a single module, in the future
-/// module, in the future we will likely have multi-module and
+/// we will likely have multi-module and multi-file justfiles.
-/// multi-file justfiles.
 ///
 /// Not all successful parses result in valid justfiles, so additional
-/// consistency checks and name resolution are performed by the
+/// consistency checks and name resolution are performed by the `Analyzer`,
-/// `Analyzer`, which produces a `Justfile` from a `Module`.
+/// which produces a `Justfile` from a `Module`.
 #[derive(Debug)]
 pub(crate) struct Module<'src> {
   /// Items in the justfile

src/name.rs

@@ -1,7 +1,7 @@
 use crate::common::*;
 
-/// A name. This is effectively just a `Token` of kind `Identifier`,
+/// A name. This is effectively just a `Token` of kind `Identifier`, but we give
-/// but we give it its own type for clarity.
+/// it its own type for clarity.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd)]
 pub(crate) struct Name<'src> {
   pub(crate) offset: usize,

src/node.rs

@@ -1,10 +1,9 @@
 use crate::common::*;
 
-/// Methods commmon to all AST nodes. Currently only used in parser
+/// Methods commmon to all AST nodes. Currently only used in parser unit tests.
-/// unit tests.
 pub(crate) trait Node<'src> {
-  /// Construct an untyped tree of atoms representing this Node. This
+  /// Construct an untyped tree of atoms representing this Node. This function,
-  /// function, and `Tree` type, are only used in parser unit tests.
+  /// and `Tree` type, are only used in parser unit tests.
   fn tree(&self) -> Tree<'src>;
 }
 

src/parser.rs

@@ -4,24 +4,20 @@ use TokenKind::*;
 
 /// Just language parser
 ///
-/// The parser is a (hopefully) straightforward recursive descent
+/// The parser is a (hopefully) straightforward recursive descent parser.
-/// parser.
 ///
-/// It uses a few tokens of lookahead to disambiguate different
+/// It uses a few tokens of lookahead to disambiguate different constructs.
-/// constructs.
 ///
-/// The `expect_*` and `presume_`* methods are similar in that they
+/// The `expect_*` and `presume_`* methods are similar in that they assert the
-/// assert the type of unparsed tokens and consume them. However, upon
+/// type of unparsed tokens and consume them. However, upon encountering an
-/// encountering an unexpected token, the `expect_*` methods return an
+/// unexpected token, the `expect_*` methods return an unexpected token error,
-/// unexpected token error, whereas the `presume_*` tokens return an
+/// whereas the `presume_*` tokens return an internal error.
-/// internal error.
 ///
-/// The `presume_*` methods are used when the token stream has been
+/// The `presume_*` methods are used when the token stream has been inspected in
-/// inspected in some other way, and thus encountering an unexpected
+/// some other way, and thus encountering an unexpected token is a bug in Just,
-/// token is a bug in Just, and not a syntax error.
+/// and not a syntax error.
 ///
-/// All methods starting with `parse_*` parse and return a language
+/// All methods starting with `parse_*` parse and return a language construct.
-/// construct.
 pub(crate) struct Parser<'tokens, 'src> {
   /// Source tokens
   tokens: &'tokens [Token<'src>],
@@ -129,8 +125,8 @@ impl<'tokens, 'src> Parser<'tokens, 'src> {
     Err(self.internal_error("`Parser::advance()` advanced past end of token stream")?)
   }
 
-  /// Return the next token if it is of kind `expected`, otherwise,
+  /// Return the next token if it is of kind `expected`, otherwise, return an
-  /// return an unexpected token error
+  /// unexpected token error
   fn expect(&mut self, expected: TokenKind) -> CompilationResult<'src, Token<'src>> {
     if let Some(token) = self.accept(expected)? {
       Ok(token)
@@ -161,8 +157,8 @@ impl<'tokens, 'src> Parser<'tokens, 'src> {
     self.expect(Eol).map(|_| ()).expected(&[Eof])
   }
 
-  /// Return an internal error if the next token is not of kind
+  /// Return an internal error if the next token is not of kind `Identifier`
-  /// `Identifier` with lexeme `lexeme`.
+  /// with lexeme `lexeme`.
   fn presume_name(&mut self, lexeme: &str) -> CompilationResult<'src, ()> {
     let next = self.advance()?;
 
@@ -182,8 +178,7 @@ impl<'tokens, 'src> Parser<'tokens, 'src> {
     }
   }
 
-  /// Return an internal error if the next token is not of kind
+  /// Return an internal error if the next token is not of kind `kind`.
-  /// `kind`.
   fn presume(&mut self, kind: TokenKind) -> CompilationResult<'src, Token<'src>> {
     let next = self.advance()?;
 
@@ -197,8 +192,7 @@ impl<'tokens, 'src> Parser<'tokens, 'src> {
     }
   }
 
-  /// Return an internal error if the next token is not one of kinds
+  /// Return an internal error if the next token is not one of kinds `kinds`.
-  /// `kinds`.
   fn presume_any(&mut self, kinds: &[TokenKind]) -> CompilationResult<'src, Token<'src>> {
     let next = self.advance()?;
     if !kinds.contains(&next.kind) {

src/platform.rs

@@ -71,14 +71,14 @@ impl PlatformInterface for Platform {
   }
 
   fn set_execute_permission(_path: &Path) -> Result<(), io::Error> {
-    // it is not necessary to set an execute permission on a script on
+    // it is not necessary to set an execute permission on a script on windows, so
-    // windows, so this is a nop
+    // this is a nop
     Ok(())
   }
 
   fn signal_from_exit_status(_exit_status: process::ExitStatus) -> Option<i32> {
-    // The rust standard library does not expose a way to extract a signal
+    // The rust standard library does not expose a way to extract a signal from a
-    // from a windows process exit status, so just return None
+    // windows process exit status, so just return None
     None
   }
 

src/platform_interface.rs

@@ -1,8 +1,8 @@
 use crate::common::*;
 
 pub(crate) trait PlatformInterface {
-  /// Construct a command equivalent to running the script at `path`
+  /// Construct a command equivalent to running the script at `path` with the
-  /// with the shebang line `shebang`
+  /// shebang line `shebang`
   fn make_shebang_command(
     path: &Path,
     working_directory: &Path,
@@ -13,11 +13,10 @@ pub(crate) trait PlatformInterface {
   /// Set the execute permission on the file pointed to by `path`
   fn set_execute_permission(path: &Path) -> Result<(), io::Error>;
 
-  /// Extract the signal from a process exit status, if it was
+  /// Extract the signal from a process exit status, if it was terminated by a
-  /// terminated by a signal
+  /// signal
   fn signal_from_exit_status(exit_status: process::ExitStatus) -> Option<i32>;
 
-  /// Translate a path from a "native" path to a path the interpreter
+  /// Translate a path from a "native" path to a path the interpreter expects
-  /// expects
   fn to_shell_path(working_directory: &Path, path: &Path) -> Result<String, String>;
 }

src/positional.rs

@@ -1,35 +1,31 @@
 use crate::common::*;
 
-/// A struct containing the parsed representation of positional
+/// A struct containing the parsed representation of positional command-line
-/// command-line arguments, i.e. arguments that are not flags,
+/// arguments, i.e. arguments that are not flags, options, or the subcommand.
-/// options, or the subcommand.
 ///
-/// The DSL of positional arguments is fairly complex and mostly
+/// The DSL of positional arguments is fairly complex and mostly accidental.
-/// accidental. There are three possible components: overrides,
+/// There are three possible components: overrides, a search directory, and the
-/// a search directory, and the rest:
+/// rest:
 ///
 /// - Overrides are of the form `NAME=.*`
 ///
-/// - After overrides comes a single optional search directory
+/// - After overrides comes a single optional search directory argument. This is
-///   argument. This is either '.', '..', or an argument that contains
+///   either '.', '..', or an argument that contains a `/`.
-///   a `/`.
 ///
-///   If the argument contains a `/`, everything before and including
+///   If the argument contains a `/`, everything before and including the slash
-///   the slash is the search directory, and everything after is added
+///   is the search directory, and everything after is added to the rest.
-///   to the rest.
 ///
 /// - Everything else is an argument.
 ///
-/// Overrides set the values of top-level variables in the justfile
+/// Overrides set the values of top-level variables in the justfile being
-/// being invoked and are a convenient way to override settings.
+/// invoked and are a convenient way to override settings.
 ///
-/// For modes that do not take other arguments, the search directory
+/// For modes that do not take other arguments, the search directory argument
-/// argument determines where to begin searching for the justfile.
+/// determines where to begin searching for the justfile.  This allows command
-/// This allows command lines like `just -l ..` and `just ../build` to
+/// lines like `just -l ..` and `just ../build` to find the same justfile.
-/// find the same justfile.
 ///
-/// For modes that do take other arguments, the search argument is
+/// For modes that do take other arguments, the search argument is simply
-/// simply prepended to rest.
+/// prepended to rest.
 #[cfg_attr(test, derive(PartialEq, Debug))]
 pub struct Positional {
   /// Overrides from values of the form `[a-zA-Z_][a-zA-Z0-9_-]*=.*`

src/recipe.rs

@@ -2,8 +2,8 @@ use crate::common::*;
 
 use std::process::{ExitStatus, Stdio};
 
-/// Return a `RuntimeError::Signal` if the process was terminated by a
+/// Return a `RuntimeError::Signal` if the process was terminated by a signal,
-/// signal, otherwise return an `RuntimeError::UnknownFailure`
+/// otherwise return an `RuntimeError::UnknownFailure`
 fn error_from_signal(
   recipe: &str,
   line_number: Option<usize>,
@@ -120,9 +120,8 @@ impl<'src, D> Recipe<'src, D> {
         // add the shebang
         text += &evaluated_lines[0];
         text += "\n";
-        // add blank lines so that lines in the generated script
+        // add blank lines so that lines in the generated script have the same line
-        // have the same line number as the corresponding lines
+        // number as the corresponding lines in the justfile
-        // in the justfile
         for _ in 1..(self.line_number() + 2) {
           text += "\n"
         }

src/search_config.rs

@@ -3,15 +3,14 @@ use crate::common::*;
 /// Controls how `just` will search for the justfile.
 #[derive(Debug, PartialEq)]
 pub(crate) enum SearchConfig {
-  /// Recursively search for the justfile upwards from the
+  /// Recursively search for the justfile upwards from the invocation directory
-  /// invocation directory to the root, setting the working
+  /// to the root, setting the working directory to the directory in which the
-  /// directory to the directory in which the justfile is
+  /// justfile is found.
-  /// found.
   FromInvocationDirectory,
   /// As in `Invocation`, but start from `search_directory`.
   FromSearchDirectory { search_directory: PathBuf },
-  /// Use user-specified justfile, with the working directory
+  /// Use user-specified justfile, with the working directory set to the
-  /// set to the directory that contains it.
+  /// directory that contains it.
   WithJustfile { justfile: PathBuf },
   /// Use user-specified justfile and working directory.
   WithJustfileAndWorkingDirectory {

src/show_whitespace.rs

@@ -1,7 +1,6 @@
 use crate::common::*;
 
-/// String wrapper that uses nonblank characters to display spaces and
+/// String wrapper that uses nonblank characters to display spaces and tabs
-/// tabs
 pub struct ShowWhitespace<'str>(pub &'str str);
 
 impl<'str> Display for ShowWhitespace<'str> {

src/summary.rs

@@ -1,16 +1,16 @@
 //! Justfile summary creation, for testing purposes only.
 //!
-//! The contents of this module are not bound by any stability
+//! The contents of this module are not bound by any stability guarantees.
-//! guarantees. Breaking changes may be introduced at any time.
+//! Breaking changes may be introduced at any time.
 //!
-//! The main entry point into this module is the `summary` function,
+//! The main entry point into this module is the `summary` function, which
-//! which parses a justfile at a given path and produces a `Summary`
+//! parses a justfile at a given path and produces a `Summary` object, which
-//! object, which broadly captures the functionality of the parsed
+//! broadly captures the functionality of the parsed justfile, or an error
-//! justfile, or an error message.
+//! message.
 //!
-//! This functionality is intended to be used with `janus`, a tool for
+//! This functionality is intended to be used with `janus`, a tool for ensuring
-//! ensuring that changes to just do not inadvertently break or
+//! that changes to just do not inadvertently break or change the interpretation
-//! change the interpretation of existing justfiles.
+//! of existing justfiles.
 
 use std::{collections::BTreeMap, fs, io, path::Path};
 

src/tree.rs

@@ -2,10 +2,9 @@ use crate::common::*;
 
 use std::mem;
 
-/// Construct a `Tree` from a symbolic expression literal. This macro,
+/// Construct a `Tree` from a symbolic expression literal. This macro, and the
-/// and the Tree type, are only used in the Parser unit tests, as a
+/// Tree type, are only used in the Parser unit tests, as a concise notation
-/// concise notation representing the expected results of parsing a
+/// representing the expected results of parsing a given string.
-/// given string.
 macro_rules! tree {
   {
     ($($child:tt)*)
@@ -63,8 +62,7 @@ impl<'text> Tree<'text> {
     Tree::atom(format!("\"{}\"", contents.as_ref()))
   }
 
-  /// Push a child node into self, turning it into a List if it was an
+  /// Push a child node into self, turning it into a List if it was an Atom
-  /// Atom
   pub(crate) fn push(self, tree: impl Into<Tree<'text>>) -> Tree<'text> {
     match self {
       Tree::List(mut children) => {
@@ -75,8 +73,8 @@ impl<'text> Tree<'text> {
     }
   }
 
-  /// Extend a self with a tail of Trees, turning self into a List if
+  /// Extend a self with a tail of Trees, turning self into a List if it was an
-  /// it was an Atom
+  /// Atom
   pub(crate) fn extend<I, T>(self, tail: I) -> Tree<'text>
   where
     I: IntoIterator<Item = T>,

tests/working_directory.rs

@@ -20,8 +20,8 @@ const DATA: &str = "OK";
 
 const WANT: &str = "shebang: OK\nexpression: OK\ndefault: OK\nlinewise: OK\n";
 
-/// Test that just runs with the correct working directory when
+/// Test that just runs with the correct working directory when invoked with
-/// invoked with `--justfile` but not `--working-directory`
+/// `--justfile` but not `--working-directory`
 #[test]
 fn justfile_without_working_directory() -> Result<(), Box<dyn Error>> {
   let tmp = tmptree! {
@@ -46,9 +46,8 @@ fn justfile_without_working_directory() -> Result<(), Box<dyn Error>> {
   Ok(())
 }
 
-/// Test that just runs with the correct working directory when
+/// Test that just runs with the correct working directory when invoked with
-/// invoked with `--justfile` but not `--working-directory`, and
+/// `--justfile` but not `--working-directory`, and justfile path has no parent
-/// justfile path has no parent
 #[test]
 fn justfile_without_working_directory_relative() -> Result<(), Box<dyn Error>> {
   let tmp = tmptree! {
@@ -74,8 +73,8 @@ fn justfile_without_working_directory_relative() -> Result<(), Box<dyn Error>> {
   Ok(())
 }
 
-/// Test that just invokes commands from the directory in which the
+/// Test that just invokes commands from the directory in which the justfile is
-/// justfile is found
+/// found
 #[test]
 fn change_working_directory_to_search_justfile_parent() -> Result<(), Box<dyn Error>> {
   let tmp = tmptree! {
@@ -100,8 +99,8 @@ fn change_working_directory_to_search_justfile_parent() -> Result<(), Box<dyn Er
   Ok(())
 }
 
-/// Test that just runs with the correct working directory when
+/// Test that just runs with the correct working directory when invoked with
-/// invoked with `--justfile` but not `--working-directory`
+/// `--justfile` but not `--working-directory`
 #[test]
 fn justfile_and_working_directory() -> Result<(), Box<dyn Error>> {
   let tmp = tmptree! {
@@ -130,8 +129,8 @@ fn justfile_and_working_directory() -> Result<(), Box<dyn Error>> {
   Ok(())
 }
 
-/// Test that just runs with the correct working directory when
+/// Test that just runs with the correct working directory when invoked with
-/// invoked with `--justfile` but not `--working-directory`
+/// `--justfile` but not `--working-directory`
 #[test]
 fn search_dir_child() -> Result<(), Box<dyn Error>> {
   let tmp = tmptree! {
@@ -158,8 +157,8 @@ fn search_dir_child() -> Result<(), Box<dyn Error>> {
   Ok(())
 }
 
-/// Test that just runs with the correct working directory when
+/// Test that just runs with the correct working directory when invoked with
-/// invoked with `--justfile` but not `--working-directory`
+/// `--justfile` but not `--working-directory`
 #[test]
 fn search_dir_parent() -> Result<(), Box<dyn Error>> {
   let tmp = tmptree! {