Specify an encoding?

[rauschma]

I love the idea behind this library!

How about implementing the Proxy like this:

const {spawnSync} = require('child_process');
module.exports = new Proxy(
  {},
  {
    get: (_, bin) => (...args) => {
      return spawnSync(bin, args, {encoding: 'utf-8'});
    },
  }
)

A benefit of specifying an encoding is that you don’t need this code anymore: Object.assign(new String(out.stdout), out)

Are you using {...this} so that users can change the options of spawnSync()? Then I’d prefer a solution that doesn’t mutate a global object – maybe:

function createShell(userOptions = {}) {
  const options = {encoding: 'utf-8', ...userOptions};
  return new Proxy(
    {},
    {
      get: (_, bin) => (...args) => {
        return spawnSync(bin, args, {encoding: 'utf-8'});
      },
    }
  );
}

const sh = createShell();
console.log(sh.ls('-la'));

[antonmedv]

A benefit of specifying an encoding is that you don’t need this code anymore: Object.assign(new String(out.stdout), out)

This is to be able to do two this: git().status and git().trim() for example.

Are you using {...this} so that users can change the options of spawnSync()?

Yes) It's kinda a hacky way. As bin is passed as func name, args as args so we are left only with this context.

git.call({encoding: 'utf-8'}, 'status')

[rauschma]

Yes. It's kinda a hacky way.

Ah, clever! (I overlooked that in the readme.)

I thought you wanted people to add properties to the Proxy, but specifying this via .call() is a clean solution.

I’d still use 'utf-8' as (overridable) default then:

 const out = require('child_process').spawnSync(bin, args, {encoding: 'utf-8', ...this})

This is to be able to do two this: git().status and git().trim() for example.

Personally, I’d prefer simply returning out:

• Then the first expression doesn’t change.
• The second expression becomes git().stdout.trim().

But I can see why you have chosen a different approach.

[antonmedv]

This boxed string allows, also, to do this:

let out = cat('file.txt')
console.log(`Content: ${out}`)

[antonmedv]

encoding: 'utf-8'

We can add it as default for sure.

[rauschma]

If I may – I have one more question about this line:

const out = require('child_process').spawnSync(bin, args, {...this})

AFAICT, strict mode should be enabled in this module because {...this} will spread the global object if the surrounding function is function-invoked (i.e., not via .call()).

[rauschma]

Without Object.assign(), the code would be:

let out = cat('file.txt')
console.log(`Content: ${out.stdout}`)

I’d be OK with that, but it is more verbose.

[antonmedv]

Oh. Yea. This should be fixed.

[rauschma]

I’ve used tinysh some more and I now agree with you that returning strings (or something string-like) is convenient. It therefore may make sense to switch from spawnSync() to execFileSync():

• It delivers all failures (child process can’t be spawned, shell error, process killed) via exceptions. In contrast, spawnSync() only throws if spawning fails.
• It returns a string with the content of stdio (if the encoding is 'utf-8').
  • Given that all failures are handled via exceptions, returning stdio strings might be enough – we might not need the other values returned by spawnSync().

Either with execFileSync() or with spawnSync(), I’d set options.shell to true:

• You basically can’t do anything on Windows if it doesn’t have that value.
• On Unix, it’s nice if wildcards work: ls('-l', '*.txt')

In other words: I’ve used the following Proxy in my experiments and I liked how it worked:

new Proxy({}, {
  get: (_, bin) => function (...args) {
    return execFileSync(bin, args,
      {
        encoding: 'utf-8',
        shell: true,
        ...this
      }
    );
  },
})

[antonmedv]

BTW, have you seen my other library with similar goal? https://github.com/google/zx

[rauschma]

Ah, cool, I wasn’t aware you wrote zx, too! I’ve been aware of that library for a while. Quite useful.