Skip to content

Conversation

@TheLeoP
Copy link
Contributor

@TheLeoP TheLeoP commented Sep 16, 2025

Fixes the issue mentioned in #2010 (comment)

Instead of only checking this for the last line of the buffer (which previously would incorrectly exclude some regions), this PR modifies all the ranges with an end that's row-exclusive, col-0 to be row-inclusive, col-exclusive (just like for nodes that don't end with a newline).

For reference, from #2010 (comment)

I asked in the "Neovim treesitter" matrix channel and clason explained to me that, if a node contains a trailing newline, it's end_ will change from row-inclusive, col-exclusive to row-exclusive, col-0.

@TheLeoP TheLeoP marked this pull request as ready for review September 16, 2025 21:48
Copy link
Member

@echasnovski echasnovski left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

If it fixes the issue, then it looks good after suggested changes (propagated to both modules).

Also, does it have to be a check for col == 0? Or can it be only for the case when line > vim.api.nvim_buf_line_count(0) and clamp to the last line plus last column position? Probably, yes, accounting for all col == 0 is better. As per the spec, region should be end-inclusive and col starts from 1. The fact that search currently works for cases like { to = { line = 3, col = 0 } } looks more like a coincidence to me (there are no tests and documentation for it).

I briefly thought about fixing this more globally by adjusting how best region/span is searched with the following diff:

@@ -1894,7 +1894,8 @@ H.get_neighborhood = function(reference_region, n_neighbors)
   -- Compute '2d neighborhood' of (possibly empty) region
   local from_line, to_line = reference_region.from.line, (reference_region.to or reference_region.from).line
   local line_start = math.max(1, from_line - n_neighbors)
-  local line_end = math.min(vim.api.nvim_buf_line_count(0), to_line + n_neighbors)
+  local max_lines = vim.api.nvim_buf_line_count(0) + 1
+  local line_end = math.min(max_lines, to_line + n_neighbors)
   local neigh2d = vim.api.nvim_buf_get_lines(0, line_start - 1, line_end, false)
   -- Append 'newline' character to distinguish between lines in 1d case
   for k, v in pairs(neigh2d) do

However, as the case of col == 0 is not explicitly supported, fixing this in specific textobject indeed looks better. It might change in the future to allow col = 0 to mean "the whole previous line" (that's why I am posting it here now to not forget), but probably unlikely.

echasnovski added a commit that referenced this pull request Sep 18, 2025
Resolve #2013

Co-authored-by: Evgeni Chasnovski <evgeni.chasnovski@gmail.com>
@echasnovski
Copy link
Member

Had time, so decided to go ahead and make the changes outside of this PR. Hope this is okay.

@TheLeoP TheLeoP deleted the ts_fix branch September 18, 2025 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants