Skip to content

"excludeselfxib" option#20

Merged
dblock merged 5 commits intodblock:masterfrom
Ezor:master
May 14, 2016
Merged

"excludeselfxib" option#20
dblock merged 5 commits intodblock:masterfrom
Ezor:master

Conversation

@Ezor
Copy link
Copy Markdown
Contributor

@Ezor Ezor commented May 13, 2016

Sometimes you want to find if a class is unused even if it is referenced from its self XIB, the "excludeselfxib" option handle this case.

Example :

  • if this option is set to false (default) Foo.h will be marked as used if a Foo.xib reference the Foo class
  • else if this option is set to true Foo.h will be marked as unused even if a Foo.xib reference the class (and if there is no other reference to Foo.h...)

Ezor added 4 commits May 13, 2016 21:46
Example : 
- if this option is set to false Foo.h will be marked as used if a Foo.xib reference the Foo class
- else if this option is set to true Foo.h will be marked as unused even if a Foo.xib reference the class (and if there is no other reference to Foo.h...)
Example : 
- if this option is set to false Foo.h will be marked as used if a Foo.xib reference the Foo class
- else if this option is set to true Foo.h will be marked as unused even if a Foo.xib reference the class (and if there is no other reference to Foo.h...)
Comment thread bin/fui Outdated

pre do |global_options, command, options, args|
$fui = Fui::Finder.new(global_options[:path])
$fui = Fui::Finder.new(global_options[:path], global_options[:excludeselfxib])
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should refactor Finder#initialize to be (path, options = {}) because path is required, but the rest is not, and just pass global_options in without path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As i said, i'm an noobie in Ruby, how can i pass global_options without path ?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options = global_options.dup
path = options.delete(:path)
Fui::Finder.new(path, options)

@dblock
Copy link
Copy Markdown
Owner

dblock commented May 13, 2016

This is great, my comments aren't "must haves", but I'd appreciate if you could address all/some/none of them. LMK. Squash your commits, too.

@dblock
Copy link
Copy Markdown
Owner

dblock commented May 13, 2016

This also needs a README update.

@Ezor
Copy link
Copy Markdown
Contributor Author

Ezor commented May 13, 2016

Thanks, i'm a noobie in Ruby, i'm not sure to be able to address all comments but i will try !

@Ezor
Copy link
Copy Markdown
Contributor Author

Ezor commented May 13, 2016

I just push a new commit with some of your suggestions.
Can you tell me more about extracting the if part from the finder.rb ?

@dblock
Copy link
Copy Markdown
Owner

dblock commented May 14, 2016

The extracting that if is really no big deal.I was thinking the line was getting long so maybe it could have been refactored into some methods.

This is great, merging.

@dblock dblock merged commit 625fbfa into dblock:master May 14, 2016
dblock pushed a commit that referenced this pull request May 14, 2016
Added --excludeselfxib.

Example :

- if this option is set to false Foo.h will be marked as used if a Foo.xib reference the Foo class
- else if this option is set to true Foo.h will be marked as unused even if a Foo.xib reference the class (and if there is no other reference to Foo.h...)
@Ezor
Copy link
Copy Markdown
Contributor Author

Ezor commented May 14, 2016

Thanks 👍

@dblock
Copy link
Copy Markdown
Owner

dblock commented May 14, 2016

I renamed excludeselfxib to ignorexib, I think it's more logical. Also updated README and such in dd48b12. LMK how you feel about that before I make a release and it's too late ;)

@Ezor
Copy link
Copy Markdown
Contributor Author

Ezor commented May 14, 2016

Well it's a little bit confusing because it's not ignoring all XIB but only the XIB with the same name of its class but it's not a big deal !

@dblock
Copy link
Copy Markdown
Owner

dblock commented May 14, 2016

I thought about that, but I think the doc is clear and if it were to ignore all XIBs it would be called --ignorexibs, plural.

@dblock
Copy link
Copy Markdown
Owner

dblock commented May 14, 2016

I released 0.4.0 with your feature, thank you!

@Ezor
Copy link
Copy Markdown
Contributor Author

Ezor commented May 14, 2016

Yep you are right, thanks a lot

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