-
Notifications
You must be signed in to change notification settings - Fork 58
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
Extract ldiff display logic to reuse it as a lib #103
base: main
Are you sure you want to change the base?
Conversation
The ldiff "display" logic was too tightly coupled with the command line program. Let's just extract it in a distinct class method accepting all the necessary parameters. One can now use Diff::LCS::Ldiff.diff?( Diff::LCS::Ldiff::InputInfo.new('file1'), Diff::LCS::Ldiff::InputInfo.new('file2'), :unified, # or :ed or :context or :report $stdout, # or a ref to [] or to a StringIO or a File binary: nil, # or true or false if you want to be explicit lines: 3) # to customize the number of context lines The result will be `true` in case of diff and `false` otherwise. The output if any, will be appended to the output parameter.
InputInfo = Struct.new(:filename, :data, :mtime) do | ||
def initialize(filename) | ||
super(filename, ::File.read(filename), ::File.stat(filename).mtime) | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a namespaced constant to not leak constants to the global namespace, and I would replace :mtime
with :stat
and just grab .mtime
from the stat when we need it (the rest of the stat data may be useful for others in the future).
InputInfo = Struct.new(:filename, :data, :mtime) do | |
def initialize(filename) | |
super(filename, ::File.read(filename), ::File.stat(filename).mtime) | |
end | |
end | |
Diff::LCS::Ldiff::InputInfo = Struct.new(:filename, :data, :stat) do | |
def initialize(filename) | |
super(filename, ::File.read(filename), ::File.stat(filename)) | |
end | |
end |
if binary.nil? | ||
old_txt = info_old.data[0, 4096].scan(/\0/).empty? | ||
new_txt = info_new.data[0, 4096].scan(/\0/).empty? | ||
binary = !old_txt || !new_txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have merge #104 which will hopefully rebase into this nicely.
The
ldiff
"display" logic was too tightly coupled with the command line program. See #46.Let's just extract it in a distinct class method accepting all the necessary parameters.
One can now use:
The result will be
true
in case of diff andfalse
otherwise.The output if any, will be appended to the output parameter.
I also used the opportunity to refacto the format handling to use a "nice"
switch
instead of multipleif
statements.Note: I didn't add test here, as normally the
ldif
bin specs should cover the critical use cases.This should fix #46 🤞🏻