learntextvis / textkit

Command line tool for manipulating and analyzing text
MIT License
28 stars 6 forks source link

Remove dash #27

Closed vlandham closed 8 years ago

vlandham commented 8 years ago

This closes #25.

The only caveat is that this method doesn't work with @iros 's switch to using Path and allowing multiple files as input.

I think that is ok, as those functions are only the initial tokenization functions - which won't be reading from standard in anyways (most likely), and so can be expected to behave a bit differently.

iros commented 8 years ago

Nice!! Looks solid to me! Tests working ok? I added some multi file tests and I do wonder if they would work (though I think they will.)

vlandham commented 8 years ago

all the tests pass, though I think we might need to extend the tests in a different way to get to testing the input parameters well.

Perhaps just a shell script to start that runs a bunch of them? Crude... but could work. I'll look more at click to see if there are hints on how to test valid inputs properly.

On Wed, Mar 30, 2016 at 2:00 PM, Irene Ros notifications@github.com wrote:

Nice!! Looks solid to me! Tests working ok? I added some multi file tests and I do wonder if they would work (though I think they will.)

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/learntextvis/textkit/pull/27#issuecomment-203634773

iros commented 8 years ago

That makes sense. +1 to merge from me.

-- Irene

On Wed, Mar 30, 2016 at 2:03 PM, Jim Vallandingham <notifications@github.com

wrote:

all the tests pass, though I think we might need to extend the tests in a different way to get to testing the input parameters well.

Perhaps just a shell script to start that runs a bunch of them? Crude... but could work. I'll look more at click to see if there are hints on how to test valid inputs properly.

On Wed, Mar 30, 2016 at 2:00 PM, Irene Ros notifications@github.com wrote:

Nice!! Looks solid to me! Tests working ok? I added some multi file tests and I do wonder if they would work (though I think they will.)

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/learntextvis/textkit/pull/27#issuecomment-203634773

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/learntextvis/textkit/pull/27#issuecomment-203635528