feat: add a newline after commands that don't end their output with one

[chankruze]

Issue: #776

[antongolub]

@chankruze,

1. There should be a test.
2. No need to convert the buffer to string.

const eol = Buffer.from('\n', 'utf-8')

if (!entry.data.slice(-1).equals(eol)) {
  entry.data = Buffer.concat([entry.data, eol])
}

[chankruze]

@antongolub What's your opinion about this ?

This method is not compatible with the Uint8Array.prototype.slice(), which is a superclass of Buffer. To copy the slice, useUint8Array.prototype.slice().

const eol = Buffer.from('\n', 'utf-8');
if (Uint8Array.prototype.slice.call(entry.data, -1)[0] !== eol[0]) {
  entry.data = Buffer.concat([entry.data, eol]);
}

[antongolub]

cc @antonmedv

Uint8Array... Ok, we get some kind of Buffer here. Maybe we should try to keep the original proto:

const B = entry.data.constructor
const eol = B.from('\n')

if (entry.data.slice(-1).equal(eol)) {
  entry.data = B.concat([entry.data, eol])
}

Does Uint8Array have own static concat helper? It does not.

[antongolub]

Well, it seems too verbose to modify the value. Let's keep the initial approach. Just move this helper to util.ts and add a test plz.

export function ensureEol (buffer: Buffer): Buffer {
  if (buffer.toString('utf8', buffer.length - 1) !== '\n') {
    return Buffer.concat([buffer, Buffer.from('\n')])
  }

  return buffer
}

[antongolub]

buffer.toString('utf8', buffer.length - 1) bothers me. I don't know how it works inside. If it is just a shortcut for .toString('utf8').slice() we could get a doubled mem usage here.

[chankruze]

@antongolub sure! will do that.

1. buffer.toString('utf8') converts the entire Buffer object buffer into a string using the UTF-8 encoding.
2. buffer.length - 1 calculates the index of the last character in the Buffer.

So, buffer.toString('utf8', buffer.length - 1) will start the conversion from the character at the position buffer.length - 1.

It's not equivalent to toString('utf8').slice() because slice() creates a new string by extracting a section of the original string, which would indeed duplicate memory usage. However, toString('utf8', buffer.length - 1) directly converts from the given index to the end of the buffer without creating a new string instance.

I think toString('utf8', buffer.length - 1) may be slightly more efficient because it doesn't involve creating an intermediate string that would contain the entire contents of the Buffer.

[chankruze]

@antongolub what about this approach ?

export function ensureEol(buffer: Buffer): Buffer {
  if (!buffer.toString('utf8').endsWith('\n')) {
    return Buffer.concat([buffer, Buffer.from('\n')])
  }

  return buffer
}

I'm currently writing test. Just to confirm, do I need to write test for both this function and the core behavior or just the core test is good enough ?

[antongolub]

1. buffer.toString('utf8').endsWith('\n') creates a new string same size as the buffer.
2. unit test for ensureEol is enough for now

[chankruze]

@antongolub please review the changes and comment if anything needs to be done.

[antongolub]

mention the method

[antongolub]

lgtm