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

Feature/readded argv #11

Merged
merged 3 commits into from
Nov 3, 2017
Merged

Feature/readded argv #11

merged 3 commits into from
Nov 3, 2017

Conversation

lindt
Copy link
Member

@lindt lindt commented Nov 3, 2017

@johngluckmdsol , I needed ARGV again, cause found, that it is not working in our scripts anymore.
Should have the same behaviour as before, except, that -f is not needed anymore.

I merge it now, but please check it, John. Thanks.

@lindt lindt merged commit 816d080 into master Nov 3, 2017
@johngluckmdsol
Copy link
Contributor

Okay, but IMO, that behavior was a defect because it's not globbing as a regular user would expect it to, so I'm inclined to say your scripts are incorrect. I will take a look and give you a more detailed analysis if pertinent before I make any judgements :)

@johngluckmdsol
Copy link
Contributor

Now I remember. Here are my notes:

We no longer use command-line expansion through ARGV to get the file list because this doesn't work in all cases (/**/* vs dir/*). This would be a bug fix That said, since we bugfixed the cli but all other changes should be backward compatible, we bumped to 0.7.0., which I believe is in accordance with semantic versioning, even though the usage has changed (in that command line arg now requires -f '<path to file/s>'

That means all you have to do to fix your scripts is surround pass -f with single-quotes. As you have it now, you've switched back to unix globbing instead of Ruby globbing, which is superior. Since I have to manage a lot of users, this is going to be confusing for them. They will expect ruby globbing and will be upset with the linter misses files

@lindt
Copy link
Member Author

lindt commented Nov 3, 2017

ok, but gherkin_lint '**/*.feature' and gherkin_lint -f '**/*.feature' would be the same.....so it is enough to put the ruby globbing around?

if have so many users, we also could support additionally the -f-behaviour.

@johngluckmdsol
Copy link
Contributor

johngluckmdsol commented Nov 3, 2017

The problem with **/*.feature is that it expands before getting into ruby and doesn't catch the files in the parent directory. I just think that's a bug. It's not obvious that's going to happen. In fact, I didn't even know that was how Unix operated with ** until I wrestled with this for 2 hours. But even if I had, I assume that when I pass an arg like this into Ruby, it is treated as a string. It feels to me like it should be a string so as to avoid Unix interpreting it before it gets to the main execution.

So no, it's not the same

gherkin_lint **/*.feature and gherkin_lint -f '**/*.feature' produce different output, the difference being that the former won't list files in the parent directory. That's why I think it's a bug

@lindt
Copy link
Member Author

lindt commented Nov 3, 2017

gherkin_lint **/*.feature and gherkin_lint -f '**/*.feature' is not the same, but
gherkin_lint '**/*.feature' and gherkin_lint -f '**/*.feature' is.

So I would add the ruby globbing around any file passed. And also additionally readd the --file syntax even it could be done without it to support your users?

@johngluckmdsol
Copy link
Contributor

Okay. I'm good with that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants