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

Support multiple files from the CLI command #110

Closed
wants to merge 1 commit into from

Conversation

schneems
Copy link
Collaborator

@schneems schneems commented Nov 9, 2021

Close #109

This change allows multiple files to be passed into the CLI with one call:

$ ./exe/dead_end **/*.rb
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK

Close #109

This change allows multiple files to be passed into the CLI with one call:

```
$ ./exe/dead_end **/*.rb
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
```
@schneems
Copy link
Collaborator Author

schneems commented Nov 9, 2021

@bkuhlmann can you take a look. Specifically, I'm interested in what kind of output you might be expecting. This is a naive implementation. It executes each file in a loop and outputs as if each call was independent.

It's a little weird to see all those Syntax Ok calls. When a file has a syntax error in it, it will be a bit more readable as it has the filename in the output. I would look like:

Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK
Syntax OK

--> /private/tmp/scratch.rb

Unmatched `end', missing keyword (`do', `def`, `if`, etc.) ?

  1  class Dog
  2    def speak
❯ 3      @sounds.each |sound|
❯ 5      end
  6    end
  7  end

Syntax OK
Syntax OK
Syntax OK
Syntax OK

But that's still not great.

I think alternatively we could do something like:

Syntax OK (38 files) 

and

--> /private/tmp/scratch.rb

Unmatched `end', missing keyword (`do', `def`, `if`, etc.) ?

  1  class Dog
  2    def speak
❯ 3      @sounds.each |sound|
❯ 5      end
  6    end
  7  end

1 failure (38 files)

Though I'm not sure if we want to also extend the same output to when there is only one file being checked:

Syntax OK (1 file) 

And

--> /private/tmp/scratch.rb

Unmatched `end', missing keyword (`do', `def`, `if`, etc.) ?

  1  class Dog
  2    def speak
❯ 3      @sounds.each |sound|
❯ 5      end
  6    end
  7  end

1 failure (1 file)

Recap

  • I'm looking for feedback on the display output
    • For the case of multiple files (success and failure)
    • For the case of N=1 files (success and failure)

@bkuhlmann
Copy link

bkuhlmann commented Nov 9, 2021

🙇 I cloned your feature branch and installed the updated version of your DeadEnd gem locally. I had to tweak your dead_end.gemspec with the following to make it install locally because the lib files were not being installed properly (💡this is something Gemsmith makes easy 😛):

spec.bindir = "exe"
spec.executables << "dead_end"
spec.files = Dir["lib/**/*"]
spec.require_paths = ["lib"]

Then I ran with your modifications on my demo project. Here are the full steps to recreate:

gem install gemsmith
gemsmith --generate demo --cli
cd demo
dead_end **/*.rb

This yielded the following results:

2021-11-08_20-27-52-iTerm2

I agree with your thoughts above, I think it'd be neat to print this instead:

Syntax OK (6 files inspected)

It would also be neat to say this too:

# Zero use case
Syntax OK (0 files inspected)

# One file use case
Syntax OK (1 file inspected)

A couple reasons why the above is nice:

  • Less burden on you, as a maintainer, to have to deal with any conditional logic. Just print the count each time.
  • Makes the output consistent which is nice. Kind of like how Rubocop or Git Lint works.

💡 You could use my String#pluralize refinement to handle pluralization calculations if you wanted.


One final thought -- in case it helps -- in the situation where you detect no issues, only printing the total count is great. In the case that errors are detected, I think it'd be neat to print each violation with maybe a summary at the bottom. Example:

Syntax OK (5 files inspected, 3 errors detected)

Anyway, hope that helps.

@schneems schneems closed this Mar 9, 2023
@schneems
Copy link
Collaborator Author

schneems commented Mar 9, 2023

I’m still interested, but no actively working on this. May reopen in the future.

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

Successfully merging this pull request may close these issues.

Support multiple files from the CLI (support dir globs)
2 participants