-
Notifications
You must be signed in to change notification settings - Fork 1
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
Handle COPY commands as per open-vcdiff implementation. #2
Conversation
@@ -13,7 +13,7 @@ class Decoder | |||
attr_accessor :dictionary | |||
|
|||
def initialize(dictionary) | |||
@dictionary = File.read(dictionary, mode: "rb") | |||
@dictionary = File.new(dictionary, 'rb').read |
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 change should be taken out.
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.
Will do. I added this before the last PR.
@pwiebe could you add tests covering the new functionality? |
@@ -132,6 +133,7 @@ def process_delta_encoding(source_window, delta_encoding) | |||
target_window << (add_run_data[add_run_index, instruction_size_1]).pack("C*") | |||
add_run_index += instruction_size_1 | |||
when CodeTable::COPY | |||
|
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'd rather this line not be added.
Thanks for the PR! I really like that this is implementing more of the VCDIFF spec! |
I'll get Travis integrated through a separate PR so I can get build checking on branches. |
👍 Phil! |
@pwiebe could you squash these 3 commits and rebase them off the latest |
@@ -159,7 +160,7 @@ def process_delta_encoding(source_window, delta_encoding) | |||
# same[(m - (s_near+2))*256 + b]". | |||
# | |||
|
|||
here = target_window.length - 1 | |||
here = window.source_data_length + target_window.length |
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.
There's a rogue space at the end of this line.
…RFC 3284 concerning the boundary between S and T).
Any movement on this? |
I missed that the commits had been squashed. I'll get to looking at this again soon! |
Handle COPY commands as per open-vcdiff implementation.
The COPY command can copy data from:
The original implementation assumed source only.