adding wc unix command #809

jumson · 2018-01-06T20:34:02Z

I added partial functionality of the Unix 'wc' command.
from the man page:

wc - Print newline, word, and byte counts for each FILE, 
and a total line if more than one FILE is specified.  A word 
is a non-zero-length sequence of characters delimited by 
white space.

and I implemented these options:

        -m, print the character counts
        -l, print the newline counts
        -w, print the word counts

I made a test file based on tail.js test, but I'm not sure how to use it.

I tested it alongside wc in bash and it gave similar results for single and multiple files.

bytes is awhole other animal..will need another module and perhaps will do it in a later version

copied tail for now

documentation updated too

jumson · 2018-01-07T15:36:59Z

This just occurred to me...why create any new command implementations in javascript when the shelljs/exec command exists?
regarding this pull request, does not:

shell.wc('file.txt')

and

shell.exec('wc file.txt')

accomplish the same thing?

jumson · 2018-01-07T16:38:25Z

In response to myself -- the time to execute these commands on windows nodejs are below. There is a clear lag when using shelljs\exec function, compared to both the shelljs\wc function I just made, and simply using nodejs' child process, as well as the regule wc executable provided in indows and ubuntu (not WSL)


PS C:\Users\xx> Measure-Command {wc .\notes-uname.txt}
TotalSeconds      : 0.0386133
TotalMilliseconds : 38.6133

PS C:\Users\xx> Measure-Command {node .\child.js}
TotalSeconds      : 1.1918551
TotalMilliseconds : 1191.8551

PS C:\Users\xx> Measure-Command {node .\e.js}
TotalSeconds      : 1.2602646
TotalMilliseconds : 1260.2646

PS C:\Users\xx> Measure-Command {node .\wc.js}
TotalSeconds      : 0.2618337
TotalMilliseconds : 261.8337

and ubuntu:

root@xx# time wc notes-uname.txt
  39  131 1244 notes-uname.txt
real    0m0.020s
user    0m0.000s
sys     0m0.004s

root@xx# time node child.js
real    0m0.266s
user    0m0.116s
sys     0m0.020s

root@xx# time node e.js
real    0m1.216s
user    0m0.284s
sys     0m0.096s

root@xx# time node wc.js
real    0m1.020s
user    0m0.216s
sys     0m0.032s

nfischer · 2018-01-11T06:12:44Z

There is a clear lag when using shelljs\exec function

Among other reasons, shell.exec() needs to execute a system call (something like execvp), and that has associated overhead. JavaScript implementations don't need that particular syscall, which gives a slight advantage.

why create any new command implementations in javascript

Another reason is to make cross-platform implementations 😄

nfischer · 2018-01-11T06:13:03Z

.gitignore

 # OS X
 .DS_Store

+file1.txt


Don't change gitignore

nfischer · 2018-01-11T06:13:24Z

commands.js

+    'uniq',
+    'wc',
+    'which',
+];


Files should end in a newline

nfischer · 2018-01-11T06:14:49Z

test/wc.js

+        .reverse()
+        .concat(bottomOfFile1.slice(0, 4).reverse())
+        .join('\n') + '\n');
+});


Files should end in a newline

nfischer · 2018-01-11T06:15:00Z

src/wc.js

+
+    return wc.join('\n');
+}
+module.exports = _wc;


Files should end in a newline

nfischer · 2018-01-11T06:15:55Z

src/wc.js

+
+    var idx = 1;
+
+    /*  - from tail.js, which I'm using as a template. Not sure I need this section


You can remove this section

nfischer · 2018-01-11T06:33:38Z

src/wc.js

+}
+
+function getChars(data) {
+    return data.length.toString();


I think this is actually wrong (I'm looking at MDN):

uses a single 16-bit code unit to represent the most common characters, but needs to use two code units for less commonly-used characters, so it's possible for the value returned by length to not match the actual number of characters in the string.

Is there a way to guarantee that two-codepoint-long characters only count as a single character each?

I went back and forth on this a lot. It's been a while (in grad school time) so my mind has been elsewhere.
There is a complication in determining the actual encoding and how that is counted. I also wanted to implement the "bytes" option -- anyways, I'll relook this as I was assuming the .length property was sufficient.

I think i'll read the file into a buffer and work on the buffer (instead of reading it in as UTF-8, as it currently stand), possibly first determining it's encoding, then moving on from there. That might require another library, we shall see.

some more initial thoughts -- since i'm already reading this stream in as a utf-8, it should be in 'chunks' that made a 16-bit code unit into a character that .length property will count as one. Here is a demonstration, using the REPL:

> const buf5 = Buffer.from('tést'); undefined > buf5 <Buffer 74 c3 a9 73 74> > buf5.toString() 'tést' > buf5.toString('utf-8') 'tést' > buf5.toString('hex') '74c3a97374' > buf5.length 5 > buf5.toString().length 4

Reference: https://nodejs.org/api/buffer.html#buffer_buffer

also, based on this conversation, I think i'll implement the '--byte' option, which may be useful

I think you need to test with a different character. Try '🎉' instead. 'é' can be encoded in a single utf-16 code unit (2 bytes).

I think i'll implement the '--byte' option, which may be useful

Yeah, that's fine, as long as we get the correct bytes of the file, not the bytes of the JS string.

Also, I'm not sure what the correct behavior is when the file is utf-16 encoded. Unix wc doesn't seem to give correct answers.

nfischer · 2018-01-11T06:38:20Z

src/wc.js

+        if (options.charCount || options.lineCount || options.wordCount) {
+            //modify wc accordingly
+
+            if (options.lineCount) temp += `${thisLines} `;


Use regular strings (single quotes)

nfischer · 2018-01-11T06:38:49Z

src/wc.js

+        'w': 'wordCount',
+        */
+
+        var temp = '';


A better variable name might be output or formattedInfo

nfischer · 2018-01-11T06:40:11Z

src/wc.js

+
+    if (moreThan1) wc.push(`${totalLines} ${totalWords} ${totalChars} total`);
+
+    return wc.join('\n');


We probably want to end this with a trailing newline. I think most other commands follow this pattern.

nfischer · 2018-01-11T06:41:03Z

src/wc.js

+        files.unshift('-');
+    }
+
+    var moreThan1 = false;


You can remove this variable, just inline files.length > 1 down below.

jumson · 2018-01-11T13:22:27Z

@nfischer thanks for the feedback, i'll probably make adjustments this weekend. (also, this is my first pull request so...noob errors will abound I'm sure)

nfischer · 2018-01-12T05:15:47Z

@jumson no worries, and thanks for the pull request! It looks pretty good overall.

Also, I think npm run lint will flag some style issues.

deleted the README, then: npm run gendocs

initial man wc data deleted

duplication / inefficient way of doing things

formattedOutput

VS Code does not like this! probably a setting I can change. Notepadd++ works fine.

This reverts commit be592bf.

This reverts commit 5040cfc.

also, using Buffers instead ofUTF-8 Strings

included method from: http://blog.jonnew.com/posts/poo-dot-length-equals-two

also, now it prints the same output as UNIX wc

jumson

@nfischer, I think I've accounted for all changes now, except, regarding this comment:

Put a space after the comment character....

when I did that,

// @ docmentation stuff

npm run gendocs failed to work (returned no errors though).
When I put it back, it worked:

//@ documentation stuff

jumson · 2018-01-13T04:01:49Z

src/wc.js

+    var regexSplit = /\s/;
+    var wordsInitial = data.split(regexSplit); //array
+
+    while (wordsInitial.indexOf('') !== -1) {


agreed - certainly more elegant. I've not used the filer() before.

nfischer · 2018-01-16T02:04:43Z

README.md

+Examples:
+
+```javascript
+var counts = wc('file.txt'); //returns lineCount wordCount byteCount


@jumson What I mean is to change "//returns" into "// returns"

nfischer · 2018-01-16T02:05:27Z

@jumson it looks like you have test failures, can you check those?

nfischer

FYI if you open src/wc.js in vim and save the file, it will automatically make sure it ends in a newline.

nfischer · 2018-01-18T07:18:48Z

test/wc.js

+        .concat(bottomOfFile1.slice(0, 4).reverse())
+        .join('\n') + '\n');
+});
+


Remove extra newline (just delete line 146)

I opened #816 to catch this automatically. Once that PR lands, you can rebase off master and fix this with npm run lint -- --fix.

nfischer · 2018-01-18T07:19:29Z

src/wc.js

+    return wc.join('\n');
+}
+
+module.exports = _wc;


Still needs a newline at the end of line 169.

@jumson actually, just run npm run lint to catch this stuff. npm run lint -- --fix will fix it.

This enforces that files should not end with blank lines (i.e. more than one trailing newline). The `eol-last` rule (inherited from airbnb's config) already enforces that files end in at least one trailing newline, so adding this rule enforces that it ends in exactly 1. See https://eslint.org/docs/rules/no-multiple-empty-lines and https://eslint.org/docs/rules/eol-last for more information. Inspired by #809.

* chore(lint): don't allow excess trailing newlines This enforces that files should not end with blank lines (i.e. more than one trailing newline). The `eol-last` rule (inherited from airbnb's config) already enforces that files end in at least one trailing newline, so adding this rule enforces that it ends in exactly 1. See https://eslint.org/docs/rules/no-multiple-empty-lines and https://eslint.org/docs/rules/eol-last for more information. Inspired by #809. * add maxBOF

nfischer · 2018-07-04T01:29:25Z

@jumson are you still working on this?

I think this is a better candidate for a shelljs plugin than a core method. Would you be interested in rewriting this in a separate repo as a plugin?

nfischer · 2018-07-10T06:39:06Z

Closing due to lack of response (also, I think this is better implemented as a 3rd party plugin).

jumson added 16 commits January 6, 2018 08:08

creating a few more commands - initial shells

7ba5240

changing tail to wc

dae6849

body done, build functions

bdeef35

bytes to chars

07b5349

bytes is awhole other animal..will need another module and perhaps will do it in a later version

update with generate-doc.js

5040cfc

added wc

7b680dc

fixed export label

588d8ab

initial testin

c851ded

copied tail for now

rearranging stuff to match linux wc

5db99d5

added total lines for multi files

532f46e

addd total for all fields

fb72cc0

works just like wc

9bb09ad

updated test

a8198eb

totals all work out, and documetnation

d91a7e6

documentation updated too

update

e6df478

adding to gitignore

68d836a

nfischer requested changes Jan 11, 2018

View reviewed changes

jumson added 8 commits January 12, 2018 19:00

fixed gitignore

3368632

fixed README

be592bf

deleted the README, then: npm run gendocs

deleted comments

58f0877

initial man wc data deleted

deleting the .toString()s and the parseInts

e3d7ccd

duplication / inefficient way of doing things

using .filter() now in getWords

90cde84

fixed comments

288b033

moreThan1 deleted

bf6726a

changed temp var to

865521d

formattedOutput

jumson added 13 commits January 12, 2018 20:32

changed the way formattedOutput is built

212e7bd

adding trailing newline

2fa5308

adding newlines

a6ee2f8

VS Code does not like this! probably a setting I can change. Notepadd++ works fine.

adding newline

bb12eb4

Revert "fixed README"

c23166e

This reverts commit be592bf.

Revert "update with generate-doc.js"

02ad178

This reverts commit 5040cfc.

resetting README.js

053aabe

moved comments and start getBytes

e08800b

adding getBytes functionality

978e2b6

also, using Buffers instead ofUTF-8 Strings

updating for thisBytes

e468b37

implenenting byte count

bb4ebf1

included method from: http://blog.jonnew.com/posts/poo-dot-length-equals-two

completed implemented bytecounts

cf8a5b8

also, now it prints the same output as UNIX wc

fixed doc comments

cee53ad

jumson commented Jan 13, 2018

View reviewed changes

newline at end did not appear to show up. adding another

91c29e0

nfischer reviewed Jan 16, 2018

View reviewed changes

nfischer requested changes Jan 18, 2018

View reviewed changes

nfischer mentioned this pull request Jan 18, 2018

chore(lint): don't allow excess trailing newlines #816

Merged

nfischer closed this Jul 10, 2018


		var idx = 1;

		/* - from tail.js, which I'm using as a template. Not sure I need this section


		if (moreThan1) wc.push(`${totalLines} ${totalWords} ${totalChars} total`);

		return wc.join('\n');

adding wc unix command #809

adding wc unix command #809

Uh oh!

Conversation

jumson commented Jan 6, 2018

Uh oh!

jumson commented Jan 7, 2018

Uh oh!

jumson commented Jan 7, 2018

Uh oh!

nfischer commented Jan 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jumson commented Jan 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nfischer commented Jan 12, 2018

Uh oh!

jumson left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nfischer commented Jan 16, 2018

Uh oh!

nfischer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nfischer commented Jul 4, 2018

Uh oh!

nfischer commented Jul 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jumson commented Jan 11, 2018 •

edited

Loading

jumson left a comment •

edited

Loading