Fix: Off by 1 Error in Old Sha #202
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Fixes #193. @jakubbortlik Would you please confirm this fixes your problem? |
harrisoncramer
commented
Feb 28, 2024
Comment on lines
-45
to
+46
| for hunk_line_index = found_hunk.old_line, hunk.old_line + hunk.old_range - 1, 1 do | ||
| for hunk_line_index = found_hunk.old_line, hunk.old_line + hunk.old_range, 1 do |
There was a problem hiding this comment.
This was the actual bug I believe...
harrisoncramer
commented
Feb 28, 2024
Comment on lines
+184
to
+213
| local function get_modification_type_from_new_sha(new_line, hunks, all_diff_output) | ||
| if new_line == nil then | ||
| return nil | ||
| end | ||
| return List.new(hunks):find(function(hunk) | ||
| local new_line_end = hunk.new_line + hunk.new_range | ||
| local in_new_range = new_line >= hunk.new_line and new_line <= new_line_end | ||
| local is_range_zero = hunk.new_range == 0 and hunk.old_range == 0 | ||
| return in_new_range and (is_range_zero or line_was_added(new_line, hunk, all_diff_output)) | ||
| end) and "added" or "bad_file_unmodified" | ||
| end | ||
|
|
||
| ---@param old_line number|nil | ||
| ---@param new_line number|nil | ||
| ---@param hunks Hunk[] | ||
| ---@param all_diff_output table | ||
| ---@return string|nil | ||
| local function get_modification_type_from_old_sha(old_line, new_line, hunks, all_diff_output) | ||
| if old_line == nil then | ||
| return nil | ||
| end | ||
|
|
||
| return List.new(hunks):find(function(hunk) | ||
| local old_line_end = hunk.old_line + hunk.old_range | ||
| local new_line_end = hunk.new_line + hunk.new_range | ||
| local in_old_range = old_line >= hunk.old_line and old_line <= old_line_end | ||
| local in_new_range = old_line >= hunk.new_line and new_line <= new_line_end | ||
| return (in_old_range or in_new_range) and line_was_removed(old_line, hunk, all_diff_output) | ||
| end) and "deleted" or "unmodified" | ||
| end |
There was a problem hiding this comment.
Using the List class makes this code much easier to understand IMO
Hi, yes, in the |
harrisoncramer
added a commit
that referenced
this pull request
Mar 3, 2024
This MR is a #MAJOR breaking change to the plugin. While the plugin will continue to work for users with their existing settings, they will be informed of outdated configuration (diagnostics and signs have been simplified) the next time they open the reviewer. Fix: Trim trailing slash from custom URLs Update: .github/CONTRIBUTING.md, .github/ISSUE_TEMPLATE/bug_report.md Feat: Improve discussion tree toggling (#192) Fix: Toggle modified notes (#188) Fix: Toggle discussion nodes correctly Feat: Show Help keymap in discussion tree winbar Fix: Enable toggling nodes from the note body Fix: Enable toggling resolved status from child nodes Fix: Only try to show emoji popup on note nodes Feat: Add keymap for toggling tree type Fix: Disable tree type toggling in Notes Fix Multi Line Issues (Large Refactor) (#197) Fix: Multi-line discussions. The calculation of a range for a multiline comment has been consolidated and moved into the location.lua file. This does not attempt to fix diagnostics. Refactor: It refactors the discussions code to split hunk parsing and management into a separate module Fix: Don't allow comments on modified buffers #194 by preventing comments on the reviewer when using --imply-local and when the working tree is dirty entirely. Refactor: It introduces a new List class for data aggregation, filtering, etc. Fix: It removes redundant API calls and refreshes from the discussion pane Fix: Location provider (#198) Fix: add nil check for Diffview performance issue (#199) Fix: Switch Tabs During Comment Creation (#200) Fix: Check if file is modified (#201) Fix: Off-By-One Issue in Old SHA (#202) Fix: Rebuild Diagnostics + Signs (#203) Fix: Off-By-One Issue in New SHA (#205) Fix: Reviewer Jumps to wrong location (#206) BREAKING CHANGE: Changes configuration of diagnostics and signs in the setup call.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.