Review fixes

README.md

@@ -64,8 +64,8 @@ section below.
 Filer also supports node's Path module. See the [Filer.Path](#FilerPath) section below.
 
 In addition, common shell operations (e.g., rm, touch, cat, etc.) are supported via the
-`FileSystemShell` object, which can be used with a `FileSystem`.  See the[Filer.FileSystemShell](#FileSystemShell)
+`FileSystemShell` object, which can be obtained from, and used with a `FileSystem`.
-section below.
+See the[FileSystemShell](#FileSystemShell) section below.
 
 ### API Reference
 
@@ -921,28 +921,27 @@ fs.open('/myfile', 'r', function(err, fd) {
 });
 ```
 
-### Filer.FileSystemShell<a name="FileSystemShell"></a>
+### FileSystemShell<a name="FileSystemShell"></a>
 
 Many common file system shell operations are available by using a `FileSystemShell` object.
-The `FileSystemShell` is used in conjuction with a `FileSystem`, and provides augmented
+The `FileSystemShell` is obtained from, and used in conjuction with a `FileSystem`,
-features. Many separate `FileSystemShell` objects can exist per `FileSystem`, but each
+and provides augmented features. Many separate `FileSystemShell` objects can exist per
-`FileSystemShell` is bound to a single instance of a `FileSystem` for its lifetime.
+`FileSystem`, but each `FileSystemShell` is bound to a single instance of a `FileSystem`
+for its lifetime.
 
-There are two ways to create a `FileSystemShell` object:
+A `FileSystemShell` is created using the `FileSystem.Shell()` function:
 
 ```javascript
-// Method 1: obtain a shell from an existing fs
 var fs = new Filer.FileSystem();
 var sh = fs.Shell(options);
-
+var sh2 = fs.Shell(options);
-// Method 2: create a shell, passing in an existing fs
+// sh and sh2 are two separate shells, each bound to fs
-var fs = new Filer.FileSystem();
-var sh = new Filer.FileSystemShell(fs, options);
 ```
 
 The `FileSystemShell` can take an optional `options` object. The `options` object
 can include `env`, which is a set of environment variables. Currently supported variables
-include `TMP` (the path to the temporary directory), and `PATH` (the list of known paths):
+include `TMP` (the path to the temporary directory), and `PATH` (the list of known paths) and
+others may be added in the future. You can also add your own, or update existing variables.
 
 ```javascript
 var fs = new Filer.FileSystem();
@@ -952,6 +951,8 @@ var sh = fs.Shell({
     PATH: '/one:/two'
   }
 });
+var tempPath = sh.env.get('TMP');
+sh.env.set('TMP', '/newtempdir');
 ```
 
 NOTE: unless otherwise stated, all `FileSystemShell` methods can take relative or absolute
@@ -963,22 +964,23 @@ of a current working directory.
 
 A `FileSystemShell` has a number of properties, including:
 * `fs` - (readonly) a reference to the bound `FileSystem`
-* `cwd` - (readonly) the current working directory (changed with `cd()`)
+* `env` - (readonly) the shell's environment. The shell's environemnt `env` object has `get(name)`
-* `env` - (readonly) the shell's environment. At runtime it will have an
+and `set(name, value)` methods.
-added `PWD` property, which is the same as `cwd`.
 
 Example:
 
 ```javascript
 var fs = new Filer.FileSystem();
 var sh = fs.Shell();
+var p = sh.env.get('PATH');
+
 // Store the current location
-var before = sh.env.PWD;
+var before = sh.pwd();
 var after;
 sh.cd('/newdir', function(err) {
   if(err) throw err;
   // Get the new location
-  after = sh.env.PWD;
+  after = sh.pwd();
 });
 ```
 
@@ -993,6 +995,7 @@ var sh = fs.Shell();
 ```
 
 * [sh.cd(path, callback)](#cd)
+* [sh.pwd()](#pwd)
 * [sh.ls(dir, [options], callback)](#ls)
 * [sh.exec(path, [options], callback)](#exec)
 * [sh.touch(path, [options], callback)](#touch)
@@ -1004,17 +1007,21 @@ var sh = fs.Shell();
 
 Changes the current working directory to the directory at `path`. The callback returns
 an error if `path` does not exist, or is not a directory. Once the callback occurs
-the shell's `cwd` property is updated to the new path (as well as `sh.env.PWD`).
+the shell's cwd is updated to the new path (you can access it via `sh.pwd()`).
 
 Example:
 
 ```javascript
 sh.cd('/dir1', function(err) {
   if(err) throw err;
-  // sh.cwd is now '/dir1'
+  // sh.pwd() is now '/dir1'
 });
 ```
 
+#### sh.pwd()<a name="pwd"></a>
+
+Returns the shell's current working directory. See [sh.cd()](#cd).
+
 #### sh.ls(dir, [options], callback)<a name="ls"></a>
 
 Get the listing of a directory, returning an array of directory entries

src/constants.js

@@ -62,7 +62,12 @@ define(function(require) {
     FS_PENDING: 'PENDING',
     FS_ERROR: 'ERROR',
 
-    SUPER_NODE_ID: '00000000-0000-0000-0000-000000000000'
+    SUPER_NODE_ID: '00000000-0000-0000-0000-000000000000',
+
+    ENVIRONMENT: {
+      TMP: '/tmp',
+      PATH: ''
+    }
   };
 
 });

src/environment.js

@@ -0,0 +1,20 @@
+define(function(require) {
+
+  var defaults = require('src/constants').ENVIRONMENT;
+
+  function Environment(env) {
+    env = env || {};
+    env.TMP = env.TMP || defaults.TMP;
+    env.PATH = env.PATH || defaults.PATH;
+
+    this.get = function(name) {
+      return env[name];
+    };
+
+    this.set = function(name, value) {
+      env[name] = value;
+    };
+  }
+
+  return Environment;
+});

src/index.js

@@ -4,7 +4,6 @@ define(function(require) {
 
   return {
     FileSystem: require('src/fs'),
-    FileSystemShell: require('src/shell'),
     Path: require('src/path')
   };
 

src/shell.js

@@ -3,15 +3,13 @@ define(function(require) {
 
   var Path = require('src/path');
   var FilerError = require('src/error');
+  var Environment = require('src/environment');
   var async = require('async');
 
   function Shell(fs, options) {
     options = options || {};
 
-    var env = options.env || {};
+    var env = new Environment(options.env);
-    env.TMP = env.TMP || '/tmp';
-    env.PATH = env.PATH || '';
-
     var cwd = '/';
 
     /**
@@ -22,33 +20,21 @@ define(function(require) {
       enumerable: true
     });
 
-    /**
-     * The current working directory (changed with `cd()`)
-     */
-    Object.defineProperty(this, 'cwd', {
-      get: function() { return cwd; },
-      enumerable: true
-    });
-
-    /**
-     * Support bash's $PWD on the env
-     */
-    Object.defineProperty(env, 'PWD', {
-      get: function() { return cwd; },
-      enumerable: true
-    });
-
     /**
      * The shell's environment (e.g., for things like
-     * path, tmp, and other env vars)
+     * path, tmp, and other env vars). Use env.get()
+     * and env.set() to work with variables.
      */
     Object.defineProperty(this, 'env', {
       get: function() { return env; },
       enumerable: true
     });
 
-    // We include `cd` on the this vs. proto so that
+    /**
-    // we can access cwd without exposing it externally.
+     * Change the current working directory. We
+     * include `cd` on the `this` vs. proto so that
+     * we can access cwd without exposing it externally.
+     */
     this.cd = function(path, callback) {
       path = Path.resolve(this.cwd, path);
       // Make sure the path actually exists, and is a dir
@@ -65,6 +51,13 @@ define(function(require) {
         }
       });
     };
+
+    /**
+     * Get the current working directory (changed with `cd()`)
+     */
+    this.pwd = function() {
+      return cwd;
+    };
   }
 
   /**
@@ -350,7 +343,7 @@ define(function(require) {
    */
   Shell.prototype.tempDir = function(callback) {
     var fs = this.fs;
-    var tmp = this.env.TMP;
+    var tmp = this.env.get('TMP');
     callback = callback || function(){};
 
     // Try and create it, and it will either work or fail

tests/spec/shell/cd.spec.js

@@ -11,7 +11,7 @@ define(["Filer", "util"], function(Filer, util) {
 
     it('should default to a cwd of /', function() {
       var shell = util.shell();
-      expect(shell.cwd).to.equal('/');
+      expect(shell.pwd()).to.equal('/');
     });
 
     it('should allow changing the path to a valid dir', function(done) {
@@ -21,10 +21,10 @@ define(["Filer", "util"], function(Filer, util) {
       fs.mkdir('/dir', function(err) {
         if(err) throw err;
 
-        expect(shell.cwd).to.equal('/');
+        expect(shell.pwd()).to.equal('/');
         shell.cd('/dir', function(err) {
           expect(err).not.to.exist;
-          expect(shell.cwd).to.equal('/dir');
+          expect(shell.pwd()).to.equal('/dir');
           done();
         });
       });
@@ -37,11 +37,11 @@ define(["Filer", "util"], function(Filer, util) {
       fs.mkdir('/dir', function(err) {
         if(err) throw err;
 
-        expect(shell.cwd).to.equal('/');
+        expect(shell.pwd()).to.equal('/');
         shell.cd('/nodir', function(err) {
           expect(err).to.exist;
           expect(err.name).to.equal('ENotDirectory');
-          expect(shell.cwd).to.equal('/');
+          expect(shell.pwd()).to.equal('/');
           done();
         });
       });
@@ -54,11 +54,11 @@ define(["Filer", "util"], function(Filer, util) {
       fs.writeFile('/file', 'file', function(err) {
         if(err) throw err;
 
-        expect(shell.cwd).to.equal('/');
+        expect(shell.pwd()).to.equal('/');
         shell.cd('/file', function(err) {
           expect(err).to.exist;
           expect(err.name).to.equal('ENotDirectory');
-          expect(shell.cwd).to.equal('/');
+          expect(shell.pwd()).to.equal('/');
           done();
         });
       });
@@ -71,10 +71,10 @@ define(["Filer", "util"], function(Filer, util) {
       fs.mkdir('/dir', function(err) {
         if(err) throw err;
 
-        expect(shell.cwd).to.equal('/');
+        expect(shell.pwd()).to.equal('/');
         shell.cd('./dir', function(err) {
           expect(err).not.to.exist;
-          expect(shell.cwd).to.equal('/dir');
+          expect(shell.pwd()).to.equal('/dir');
           done();
         });
       });
@@ -87,13 +87,13 @@ define(["Filer", "util"], function(Filer, util) {
       fs.mkdir('/dir', function(err) {
         if(err) throw err;
 
-        expect(shell.cwd).to.equal('/');
+        expect(shell.pwd()).to.equal('/');
         shell.cd('./dir', function(err) {
           expect(err).not.to.exist;
-          expect(shell.cwd).to.equal('/dir');
+          expect(shell.pwd()).to.equal('/dir');
           shell.cd('..', function(err) {
             expect(err).not.to.exist;
-            expect(shell.cwd).to.equal('/');
+            expect(shell.pwd()).to.equal('/');
             done();
           });
         });
@@ -112,7 +112,7 @@ define(["Filer", "util"], function(Filer, util) {
           var shell = fs.Shell();
           shell.cd('link', function(error) {
             expect(error).not.to.exist;
-            expect(shell.cwd).to.equal('/link');
+            expect(shell.pwd()).to.equal('/link');
             done();
           });
         });

tests/spec/shell/env.spec.js

@@ -7,9 +7,9 @@ define(["Filer", "util"], function(Filer, util) {
     it('should get default env options', function() {
       var shell = util.shell();
       expect(shell.env).to.exist;
-      expect(shell.env.TMP).to.equal('/tmp');
+      expect(shell.env.get('TMP')).to.equal('/tmp');
-      expect(shell.env.PATH).to.equal('');
+      expect(shell.env.get('PATH')).to.equal('');
-      expect(shell.env.PWD).to.equal('/');
+      expect(shell.pwd()).to.equal('/');
     });
 
     it('should be able to specify env options', function() {
@@ -21,9 +21,13 @@ define(["Filer", "util"], function(Filer, util) {
       };
       var shell = util.shell(options);
       expect(shell.env).to.exist;
-      expect(shell.env.TMP).to.equal('/tempdir');
+      expect(shell.env.get('TMP')).to.equal('/tempdir');
-      expect(shell.env.PATH).to.equal('/dir');
+      expect(shell.env.get('PATH')).to.equal('/dir');
-      expect(shell.env.PWD).to.equal('/');
+      expect(shell.pwd()).to.equal('/');
+
+      expect(shell.env.get('FOO')).not.to.exist;
+      shell.env.set('FOO', 1);
+      expect(shell.env.get('FOO')).to.equal(1);
     });
 
     it('should fail when dirs argument is absent', function(done) {
@@ -37,20 +41,17 @@ define(["Filer", "util"], function(Filer, util) {
       });
     });
 
-    it('should update the value of env.PWD when cwd changes', function(done) {
+    it('should give new value for shell.pwd() when cwd changes', function(done) {
       var fs = util.fs();
       var shell = fs.Shell();
 
       fs.mkdir('/dir', function(err) {
         if(err) throw err;
 
-        expect(shell.cwd).to.equal('/');
+        expect(shell.pwd()).to.equal('/');
-        expect(shell.env.PWD).to.equal('/');
-
         shell.cd('/dir', function(err) {
           expect(err).not.to.exist;
-          expect(shell.cwd).to.equal('/dir');
+          expect(shell.pwd()).to.equal('/dir');
-          expect(shell.env.PWD).to.equal('/dir');
           done();
         });
       });
@@ -60,13 +61,12 @@ define(["Filer", "util"], function(Filer, util) {
       var fs = util.fs();
       var shell = fs.Shell();
 
-      expect(shell.env.TMP).to.equal('/tmp');
+      expect(shell.env.get('TMP')).to.equal('/tmp');
       shell.tempDir(function(err, tmp) {
         expect(err).not.to.exist;
         shell.cd(tmp, function(err) {
           expect(err).not.to.exist;
-          expect(shell.cwd).to.equal('/tmp');
+          expect(shell.pwd()).to.equal('/tmp');
-          expect(shell.env.PWD).to.equal('/tmp');
           done();
         });
       });
@@ -80,13 +80,12 @@ define(["Filer", "util"], function(Filer, util) {
         }
       });
 
-      expect(shell.env.TMP).to.equal('/tempdir');
+      expect(shell.env.get('TMP')).to.equal('/tempdir');
       shell.tempDir(function(err, tmp) {
         expect(err).not.to.exist;
         shell.cd(tmp, function(err) {
           expect(err).not.to.exist;
-          expect(shell.cwd).to.equal('/tempdir');
+          expect(shell.pwd()).to.equal('/tempdir');
-          expect(shell.env.PWD).to.equal('/tempdir');
           done();
         });
       });
@@ -96,7 +95,7 @@ define(["Filer", "util"], function(Filer, util) {
       var fs = util.fs();
       var shell = fs.Shell();
 
-      expect(shell.env.TMP).to.equal('/tmp');
+      expect(shell.env.get('TMP')).to.equal('/tmp');
       shell.tempDir(function(err, tmp) {
         expect(err).not.to.exist;
         expect(tmp).to.equal('/tmp');