Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update line-count.R #439

Merged
merged 1 commit into from Jul 16, 2019

Conversation

Projects
None yet
3 participants
@jperkel
Copy link
Contributor

commented Jul 5, 2019

Original version incorrectly counts total line numbers if only one filename provided on command line. Removing the if (length(args) > 1) check fixes that.

@katrinleinweber katrinleinweber added the bug label Jul 6, 2019

@katrinleinweber

This comment was marked as outdated.

Copy link
Contributor

commented Jul 6, 2019

Dear @jperkel,

thank you for notifying us of this bug and suggesting a fix :-) Because I'm on travels, I may need to wait up to next weekend to reproduce & verify both.

Maybe @diyadas can have a look in the mean-time?

Kind regards,
Katrin

@diyadas

This comment was marked as outdated.

Copy link
Contributor

commented Jul 10, 2019

Hello! I have also been traveling but I will have a look tonight. Thanks both!

@diyadas

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2019

Hi, I managed to take a quick look this morning. This is indeed a bug - thanks for catching it.

The challenge question only asks for the total number of lines if more than one file is provided, so I think the precise solution would be to wrap the cat statements in question with if(length(args)>1). However, I’d also be fine with removing it entirely if the challenge text were updated to ask for a total number of lines across all files provided to line-count.R.

@katrinleinweber do you have a preference?

Update line-count.R
Incorrectly counts total line numbers if only one filename provided on command line.

@jperkel jperkel force-pushed the jperkel:patch-1 branch from d2b237c to 56f441b Jul 16, 2019

@katrinleinweber katrinleinweber merged commit 01c9351 into swcarpentry:master Jul 16, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.