search

ashfurrow / danger-ruby-swiftlint

A Danger plugin for SwiftLint.
https://rubygems.org/gems/danger-swiftlint
MIT License
203 stars 81 forks source link

Inline mode fails if filename has spaces #115

Open freak4pc opened 5 years ago

freak4pc commented 5 years ago

When setting inline_mode, there are no inline comments if the filename has a space in it.

Working example: image

Test where filename has space just fall-backs into a regular warning: image

Did you bump into this issue any where? If it's a bug I can try to tackle it.

ashfurrow commented 5 years ago

I haven't run into this, it's probably somewhere where. My guess is the problem is on this line:

https://github.com/ashfurrow/danger-ruby-swiftlint/blob/71afc40604100a48cee7b38ff736865c40f1d581/lib/danger_plugin.rb#L128

Which should be something like `.merge(path: "'#{file}'") to escape it for the command line. A unit test would be helpful here, too. Thanks for taking a look!

freak4pc commented 5 years ago

Thanks @ashfurrow - I'm not entirely sure on how to add a unit test. I noticed all Swift files in fixtures are empty - so I'm not entirely sure how the tests are constructed to fail. Any hints?

freak4pc commented 5 years ago

The more I look into this, the more I think this is actually an issue with Danger itself, and not this plugin.

daniel-beard commented 5 years ago

@freak4pc I fixed this in danger recently, can you retry with v 6.0.0 ?

freak4pc commented 5 years ago

I'll try in a few days :)

eneko commented 4 years ago

@daniel-beard, @ashfurrow It seems this issue is still present on Danger 6.1.0 with danger-ruby-swiftlint 0.23.0

In my case, Danger fails with an "invalid Dangerfile error" when the git diff contains files with spaces in their file path (some files with spaces on them are being deleted from git in the log below).

Using danger 6.1.0
357Using danger-plugin-api 1.0.0
358Using danger-auto_label 1.3.1
359Using thor 0.20.3
360Using danger-swiftlint 0.23.0
361Bundle complete! 3 Gemfile dependencies, 26 gems now installed.
362Bundled gems are installed into `./vendor/bundle`
363The command "bundle install" exited with 0.
3643.72s$ bundle exec danger
365NOTE: Inheriting Faraday::Error::ClientError is deprecated; use Faraday::ClientError instead. It will be removed in or after version 1.0
366Faraday::Error::ClientError.inherited called from /Users/travis/build/xyz/vendor/bundle/ruby/2.6.0/gems/octokit-4.14.0/lib/octokit/middleware/follow_redirects.rb:14.
367/Users/travis/build/xyz/vendor/bundle/ruby/2.6.0/gems/git-1.5.0/lib/git/diff.rb:135:in `split':  (Danger::DSLError)
368[!] Invalid `Dangerfile` file: invalid byte sequence in UTF-8
369 #  from Dangerfile:3
370 #  -------------------------------------------
371 #  
372 >  touched_files = git.modified_files + git.added_files
373 #  
374 #  -------------------------------------------

The error does not happen when files affected by the diff do not contain spaces. Not sure if this is a danger-swiftlint issue or just danger ruby.

ashfurrow commented 4 years ago

@eneko thanks for the update! Since the error is a Danger::DSLError, my guess is that it's on Danger's side. I see you already following up on https://github.com/danger/danger/issues/1042 , thank you 🙌 Please let me know what I can do to support you!

eneko commented 4 years ago

Thanks, @ashfurrow. It seems to be an issue on OctoKit (https://github.com/octokit/octokit.rb/pull/1118). Hoping it gets resolved toon. Regards.