add ignore_keys options, fixes #86 #87
Conversation
lib/hashdiff/diff.rb
Outdated
| @@ -90,6 +93,8 @@ def self.diff(obj1, obj2, options = {}, &block) | |||
|
|
|||
| opts[:prefix] = [] if opts[:array_path] && opts[:prefix] == '' | |||
|
|
|||
| opts[:ignore_keys] = [*opts[:ignore_keys]] | |||
There was a problem hiding this comment.
It's not clear what this line does.
There was a problem hiding this comment.
As a single value can be passed (sym or string) the default [] will be overridden by this and thus it is necessary to convert single args to an array with the splat.
There was a problem hiding this comment.
Thanks for explanation, a comment would be very helpful.
There was a problem hiding this comment.
Or, we can tighten the rules by requiring that ignore_keys must be an array, unless it's an established convention in Ruby libs.
There was a problem hiding this comment.
Will add a comment, as I think it is nice to have the flexibility of passing sym or string and it's a common enough idiom to splat a param that can be single object or an array.
| it 'ignores a single key' do | ||
| diff = described_class.diff(a, b, ignore_keys: :a) | ||
| diff.should == [['~', 'c', 4, 5]] | ||
| end |
There was a problem hiding this comment.
To specify which keys to ignore, it'll be good to have the choice to specify with precision. Could the ignored keys be an array of paths? That seems to complicate the specification/interface.
Will custom comparison method address the use case? https://github.com/liufengyun/hashdiff?tab=readme-ov-file#specifying-a-custom-comparison-method
There was a problem hiding this comment.
As I said in #86, a custom comparison can do this, the PR is to add a convenience method. It was my original intention to ignore only keys at the end of a path, but that is more complex, could that be a further enhancement?
An array of paths may need to deal with the :delimiter option and non-existent paths so I tried to keep it simple to start with.
There was a problem hiding this comment.
As it's useful, I think it's OK to have this simple utility option. We can avoid misunderstanding by making its intended effect more explicit in the documentation and invite users to use custom comparison for more complex usage.
| b = { a: 2, b: { d: 2, a: 7 }, c: 5 } | ||
| diff = Hashdiff.diff(a, b, ignore_keys: :a) | ||
| diff.should == [['~', 'c', 4, 5]] | ||
| ``` |
There was a problem hiding this comment.
Please refer the user to custom comparison for complex use cases (e.g., ignore keys for some path, but not all path).
So here is a PR. I've had to update some gem deps to get tests to run with Ruby 3.2/3.3, hope this is OK:
Looks like
gem 'rake', '< 11'was pinned in the Gemfile because of a change in RSpec. However, Ruby 3.2+ needs a more recent Rake - so I've bumped RSpec dep to~> 3.5as this comment suggests.Also, Rubocop needs to be >= 1.52.1 to work on Ruby 3.3 (out in < 2 weeks!) - see the PR #11942 linked on the release page. As a result
rubocop-rspecalso needed to be bumped to > 1.16.0 due to a bug there.I've fixed the resulting Rubocop errors by disabling the relevant cops in
.rubocop.yml. There are a few deprecation warnings and lots of Rubocop suggestions, but tests are green and run on Ruby 3.3.0-preview3.