Address feedback.

cli/BUILD.gn

@@ -52,9 +52,10 @@ ts_sources = [
   "../js/buffer.ts",
   "../js/build.ts",
   "../js/chmod.ts",
-  "../js/console_table.ts",
+  "../js/colors.ts",
   "../js/compiler.ts",
   "../js/console.ts",
+  "../js/console_table.ts",
   "../js/copy_file.ts",
   "../js/core.ts",
   "../js/custom_event.ts",

cli/compiler.rs

@@ -165,9 +165,11 @@ pub fn get_compiler_config(
   parent_state: &ThreadSafeState,
   _compiler_type: &str,
 ) -> Option<(String, Vec<u8>)> {
-  match (&parent_state.config_file_name, &parent_state.config) {
-    (Some(config_file_name), Some(config)) => {
-      Some((config_file_name.to_string(), config.to_vec()))
+  // The compiler type is being passed to make it easier to implement custom
+  // compilers in the future.
+  match (&parent_state.config_path, &parent_state.config) {
+    (Some(config_path), Some(config)) => {
+      Some((config_path.to_string(), config.to_vec()))
     }
     _ => None,
   }

cli/deno_dir.rs

@@ -51,6 +51,8 @@ pub struct DenoDir {
   // This splits to http and https deps
   pub deps_http: PathBuf,
   pub deps_https: PathBuf,
+  /// The active configuration file contents (or empty array) which applies to
+  /// source code cached by `DenoDir`.
   pub config: Vec<u8>,
 }
 
@@ -77,6 +79,10 @@ impl DenoDir {
     let deps_http = deps.join("http");
     let deps_https = deps.join("https");
 
+    // Internally within DenoDir, we use the config as part of the hash to
+    // determine if a file has been transpiled with the same configuration, but
+    // we have borrowed the `State` configuration, which we want to either clone
+    // or create an empty `Vec` which we will use in our hash function.
     let config = match state_config {
       Some(config) => config.clone(),
       _ => b"".to_vec(),
@@ -167,6 +173,9 @@ impl DenoDir {
 
     let gen = self.gen.clone();
 
+    // If we don't clone the config, we then end up creating an implied lifetime
+    // which gets returned in the future, so we clone here so as to not leak the
+    // move below when the future is resolving.
     let config = self.config.clone();
 
     Either::B(
@@ -485,6 +494,8 @@ fn load_cache2(
   Ok((read_output_code, read_source_map))
 }
 
+/// Generate an SHA1 hash for source code, to be used to determine if a cached
+/// version of the code is valid or invalid.
 fn source_code_hash(
   filename: &str,
   source_code: &[u8],
@@ -937,6 +948,8 @@ mod tests {
 
   #[test]
   fn test_cache_path_config() {
+    // We are changing the compiler config from the "mock" and so we expect the
+    // resolved files coming back to not match the calculated hash.
     let (temp_dir, deno_dir) = test_setup();
     let filename = "hello.js";
     let source_code = b"1+2";

cli/flags.rs

@@ -13,7 +13,9 @@ pub struct DenoFlags {
   pub log_debug: bool,
   pub version: bool,
   pub reload: bool,
-  pub config: Option<String>,
+  /// When the `--config`/`-c` flag is used to pass the name, this will be set
+  /// the path passed on the command line, otherwise `None`.
+  pub config_path: Option<String>,
   pub allow_read: bool,
   pub allow_write: bool,
   pub allow_net: bool,
@@ -154,7 +156,7 @@ pub fn parse_flags(matches: ArgMatches) -> DenoFlags {
   if matches.is_present("reload") {
     flags.reload = true;
   }
-  flags.config = matches.value_of("config").map(ToOwned::to_owned);
+  flags.config_path = matches.value_of("config").map(ToOwned::to_owned);
   if matches.is_present("allow-read") {
     flags.allow_read = true;
   }
@@ -370,7 +372,7 @@ mod tests {
     assert_eq!(
       flags,
       DenoFlags {
-        config: Some("tsconfig.json".to_owned()),
+        config_path: Some("tsconfig.json".to_owned()),
         ..DenoFlags::default()
       }
     )

cli/ops.rs

@@ -446,6 +446,7 @@ fn op_fetch_module_meta_data(
   }()))
 }
 
+/// Retrieve any relevant compiler configuration.
 fn op_compiler_config(
   state: &ThreadSafeState,
   base: &msg::Base<'_>,

cli/state.rs

@@ -52,8 +52,12 @@ pub struct State {
   pub argv: Vec<String>,
   pub permissions: DenoPermissions,
   pub flags: flags::DenoFlags,
+  /// When flags contains a `.config_path` option, the content of the
+  /// configuration file will be resolved and set.
   pub config: Option<Vec<u8>>,
-  pub config_file_name: Option<String>,
+  /// When flags contains a `.config_path` option, the fully qualified path
+  /// name of the passed path will be resolved and set.
+  pub config_path: Option<String>,
   pub metrics: Metrics,
   pub worker_channels: Mutex<WorkerChannels>,
   pub global_timer: Mutex<GlobalTimer>,
@@ -100,7 +104,8 @@ impl ThreadSafeState {
     let external_channels = (worker_in_tx, worker_out_rx);
     let resource = resources::add_worker(external_channels);
 
-    let config_file = match &flags.config {
+    // take the passed flag and resolve the file name relative to the cwd
+    let config_file = match &flags.config_path {
       Some(config_file_name) => {
         debug!("Compiler config file: {}", config_file_name);
         let cwd = std::env::current_dir().unwrap();
@@ -109,7 +114,9 @@ impl ThreadSafeState {
       _ => None,
     };
 
-    let config_file_name = match &config_file {
+    // Convert the PathBuf to a canonicalized string.  This is needed by the
+    // compiler to properly deal with the configuration.
+    let config_path = match &config_file {
       Some(config_file) => Some(
         config_file
           .canonicalize()
@@ -121,6 +128,7 @@ impl ThreadSafeState {
       _ => None,
     };
 
+    // Load the contents of the configuration file
     let config = match &config_file {
       Some(config_file) => {
         debug!("Attempt to load config: {}", config_file.to_str().unwrap());
@@ -141,7 +149,7 @@ impl ThreadSafeState {
       permissions: DenoPermissions::from_flags(&flags),
       flags,
       config,
-      config_file_name,
+      config_path,
       metrics: Metrics::default(),
       worker_channels: Mutex::new(internal_channels),
       global_timer: Mutex::new(GlobalTimer::new()),

js/colors.ts

@@ -0,0 +1,40 @@
+// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
+
+// TODO(kitsonk) Replace with `deno_std/colors/mod.ts` when we can load modules
+// which end in `.ts`.
+
+import { noColor } from "./os";
+
+interface Code {
+  open: string;
+  close: string;
+  regexp: RegExp;
+}
+
+let enabled = !noColor;
+
+function code(open: number, close: number): Code {
+  return {
+    open: `\x1b[${open}m`,
+    close: `\x1b[${close}m`,
+    regexp: new RegExp(`\\x1b\\[${close}m`, "g")
+  };
+}
+
+function run(str: string, code: Code): string {
+  return enabled
+    ? `${code.open}${str.replace(code.regexp, code.open)}${code.close}`
+    : str;
+}
+
+export function bold(str: string): string {
+  return run(str, code(1, 22));
+}
+
+export function yellow(str: string): string {
+  return run(str, code(33, 39));
+}
+
+export function cyan(str: string): string {
+  return run(str, code(36, 39));
+}

js/compiler.ts

@@ -3,6 +3,7 @@ import * as ts from "typescript";
 import * as msg from "gen/cli/msg_generated";
 import { window } from "./window";
 import { assetSourceCode } from "./assets";
+import { bold, cyan, yellow } from "./colors";
 import { Console } from "./console";
 import { core } from "./core";
 import { cwd } from "./dir";
@@ -683,19 +684,11 @@ export default function denoMain(): void {
   if (data.length) {
     const ignoredOptions = compiler.configure(path, data);
     if (ignoredOptions) {
-      if (os.noColor) {
-        console.warn(
-          `Unsupported compiler options in "${path}"\n` +
-            `  The following options were ignored:\n` +
-            `    ${ignoredOptions.join(", ")}`
-        );
-      } else {
-        console.warn(
-          `\x1b[33mUnsupported compiler options in "${path}"\x1b[39m\n` +
-            `  \x1b[36mThe following options were ignored:\x1b[39m\n` +
-            `    \x1b[1m${ignoredOptions.join("\x1b[22m, \x1b[1m")}\x1b[22m`
-        );
-      }
+      console.warn(
+        yellow(`Unsupported compiler options in "${path}"\n`) +
+          cyan(`  The following options were ignored:\n`) +
+          `    ${ignoredOptions.map((value): string => bold(value)).join(", ")}`
+      );
     }
   }
 }