Skip to content

fix(cat): make behavior more like unix#326

Merged
ariporad merged 1 commit intomasterfrom
fix-cat-semantics
Feb 1, 2016
Merged

fix(cat): make behavior more like unix#326
ariporad merged 1 commit intomasterfrom
fix-cat-semantics

Conversation

@nfischer
Copy link
Copy Markdown
Member

cat() no longer puts \n's in weird places (causing double newlines).

The behavior of cat() previously didn't make much sense, since it would stick newlines between files where they don't belong (which unix's cat does not actually do).

Example:

$ echo -n 'test1' > file1 # this file doesn't end in a newline

$ echo -n 'test2' > file2 # neither does this one

$ cat file1 file2
test1test2
$ # this prompt has no empty line above it, which means that the previous output didn't end in a newline
> // Without this fix, ShellJS would behave as follows...
> echo('test1').to('file1'); // no trailing newline
> echo('test2').to('file2'); // no trailing newline
> var s = cat('file1', 'file2');
> assert.equal(s, 'test1\ntest2'); // the newline in the middle shouldn't be there

`cat()` no longer puts '\n's in weird places (causing double newlines), and
no longer improperly strips off a trailing newline.
@nfischer nfischer added fix Bug/defect, or a fix for such a problem bash compat Compatibility issues with bash or POSIX behavior labels Jan 31, 2016
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this there? Wouldn't it actually be incorrect.

Example:

$ printf "foo\n" > foo # printf is like echo, but it properly handles `\n`.
$ printf "bar\n" > bar
$ cat foo bar
foo
bar
$ # There was a newline above this prompt.

With this line, there would be no newline at the end.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ariporad I'm not sure what you intend to be the correct behavior. I tried your example, and I see a newline above the prompt.

The line I removed would strip a trailing newline that the old code incorrectly appended. The code doesn't append newlines anymore, so it doesn't need to strip a trailing newline.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nfischer: Whoops... I apparently can't read... Never mind. (I thought you added this).

@ariporad
Copy link
Copy Markdown
Contributor

@nfischer: One comment, otherwise looks good!

@ariporad ariporad added this to the v0.6.0 milestone Jan 31, 2016
@nfischer
Copy link
Copy Markdown
Member Author

nfischer commented Feb 1, 2016

@ariporad could you clarify your comment?

@ariporad
Copy link
Copy Markdown
Contributor

ariporad commented Feb 1, 2016

LGTM!

ariporad added a commit that referenced this pull request Feb 1, 2016
fix(cat): make behavior more like unix
@ariporad ariporad merged commit ee5baf7 into master Feb 1, 2016
@ariporad ariporad deleted the fix-cat-semantics branch February 1, 2016 04:25
@nfischer
Copy link
Copy Markdown
Member Author

nfischer commented Feb 1, 2016

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bash compat Compatibility issues with bash or POSIX behavior fix Bug/defect, or a fix for such a problem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants