Skip to content

Commit

Permalink
fix(pickers): improve CRLF line splitting support for windows (#3127)
Browse files Browse the repository at this point in the history
* fix(help_tags): show help tags on windows (#3126)

On Windows, `builtin.help_tags` picker does not show any help tags.

To fix this, the following changes are needed:

1. `util.path_tail` checks unix separator `/` on Windows and leave the
original implementation intact on unix systems.

2. Line endings should be taken carefully on Windows. `vim.split` with
   only newline `\n` character as separator may result in unexpected
   crash when parsing large files. When splits on lines are needed, call
   it with `\r?\n`, or even set up a wrapper function in utils is more
   prefered.

Fixes #3126

* fix: handle cross platform line splits
  • Loading branch information
xudyang1 committed May 26, 2024
1 parent c2ce039 commit 349660c
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 14 deletions.
12 changes: 6 additions & 6 deletions lua/telescope/builtin/__internal.lua
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ internal.oldfiles = function(opts)
local results = {}

if opts.include_current_session then
for _, buffer in ipairs(vim.split(vim.fn.execute ":buffers! t", "\n")) do
for _, buffer in ipairs(utils.split_lines(vim.fn.execute ":buffers! t")) do
local match = tonumber(string.match(buffer, "%s*(%d+)"))
local open_by_lsp = string.match(buffer, "line 0$")
if match and not open_by_lsp then
Expand Down Expand Up @@ -574,7 +574,7 @@ end

internal.command_history = function(opts)
local history_string = vim.fn.execute "history cmd"
local history_list = vim.split(history_string, "\n")
local history_list = utils.split_lines(history_string)

local results = {}
local filter_fn = opts.filter_fn
Expand Down Expand Up @@ -614,7 +614,7 @@ end

internal.search_history = function(opts)
local search_string = vim.fn.execute "history search"
local search_list = vim.split(search_string, "\n")
local search_list = utils.split_lines(search_string)

local results = {}
for i = #search_list, 3, -1 do
Expand Down Expand Up @@ -690,7 +690,7 @@ internal.help_tags = function(opts)
opts.fallback = vim.F.if_nil(opts.fallback, true)
opts.file_ignore_patterns = {}

local langs = vim.split(opts.lang, ",", true)
local langs = vim.split(opts.lang, ",", { trimempty = true })
if opts.fallback and not vim.tbl_contains(langs, "en") then
table.insert(langs, "en")
end
Expand Down Expand Up @@ -729,11 +729,11 @@ internal.help_tags = function(opts)
local delimiter = string.char(9)
for _, lang in ipairs(langs) do
for _, file in ipairs(tag_files[lang] or {}) do
local lines = vim.split(Path:new(file):read(), "\n", true)
local lines = utils.split_lines(Path:new(file):read(), { trimempty = true })
for _, line in ipairs(lines) do
-- TODO: also ignore tagComment starting with ';'
if not line:match "^!_TAG_" then
local fields = vim.split(line, delimiter, true)
local fields = vim.split(line, delimiter, { trimempty = true })
if #fields == 3 and not tags_map[fields[1]] then
if fields[1] ~= "help-tags" or fields[2] ~= "tags" then
table.insert(tags, {
Expand Down
2 changes: 1 addition & 1 deletion lua/telescope/previewers/buffer_previewer.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1090,7 +1090,7 @@ previewers.highlights = defaulter(function(_)

define_preview = function(self, entry)
if not self.state.bufname then
local output = vim.split(vim.fn.execute "highlight", "\n")
local output = utils.split_lines(vim.fn.execute "highlight")
local hl_groups = {}
for _, v in ipairs(output) do
if v ~= "" then
Expand Down
6 changes: 4 additions & 2 deletions lua/telescope/previewers/utils.lua
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ local conf = require("telescope.config").values
local Job = require "plenary.job"
local Path = require "plenary.path"

local telescope_utils = require "telescope.utils"

local utils = {}

local detect_from_shebang = function(p)
local s = p:readbyterange(0, 256)
if s then
local lines = vim.split(s, "\n")
local lines = telescope_utils.split_lines(s)
return vim.filetype.match { contents = lines }
end
end
Expand All @@ -24,7 +26,7 @@ end
local detect_from_modeline = function(p)
local s = p:readbyterange(-256, 256)
if s then
local lines = vim.split(s, "\n")
local lines = telescope_utils.split_lines(s)
local idx = lines[#lines] ~= "" and #lines or #lines - 1
if idx >= 1 then
return parse_modeline(lines[idx])
Expand Down
34 changes: 29 additions & 5 deletions lua/telescope/utils.lua
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,25 @@ end)()
utils.path_tail = (function()
local os_sep = utils.get_separator()

return function(path)
for i = #path, 1, -1 do
if path:sub(i, i) == os_sep then
return path:sub(i + 1, -1)
if os_sep == "/" then
return function(path)
for i = #path, 1, -1 do
if path:sub(i, i) == os_sep then
return path:sub(i + 1, -1)
end
end
return path
end
else
return function(path)
for i = #path, 1, -1 do
local c = path:sub(i, i)
if c == os_sep or c == "/" then
return path:sub(i + 1, -1)
end
end
return path
end
return path
end
end)()

Expand Down Expand Up @@ -727,4 +739,16 @@ utils.reverse_table = function(input_table)
return temp_table
end

utils.split_lines = (function()
if utils.iswin then
return function(s, opts)
return vim.split(s, "\r?\n", opts)
end
else
return function(s, opts)
return vim.split(s, "\n", opts)
end
end
end)()

return utils
84 changes: 84 additions & 0 deletions lua/tests/automated/utils_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -307,3 +307,87 @@ describe("transform_path", function()
end, new_relpath "doc/mydoc.md", new_relpath "d/mydoc.md")
end)
end)

describe("path_tail", function()
local function assert_tails(paths)
for _, path in ipairs(paths) do
it("gets the tail of " .. path, function()
local tail = vim.fn.fnamemodify(path, ":p:t")
eq(tail, utils.path_tail(path))
end)
end
end

if jit and jit.os:lower() == "windows" then
describe("handles windows paths", function()
local paths = {
[[C:\Users\username\AppData\Local\nvim-data\log]],
[[D:\Projects\project_folder\source_code.py]],
[[E:\Music\song.mp3]],
[[/home/usuario/documents/archivo.txt]],
[[/var/www/html/index.html]],
[[/mnt/backup/backup_file.tar.gz]],
}

assert_tails(paths)
end)
elseif jit and jit.os:lower() == "linux" then
describe("handles linux paths", function()
local paths = {
[[/home/usuario/documents/archivo.txt]],
[[/var/www/html/index.html]],
[[/mnt/backup/backup_file.tar.gz]],
}

assert_tails(paths)
end)
elseif jit and jit.os:lower() == "osx" then
describe("handles macos paths", function()
local paths = {
[[/Users/Usuario/Documents/archivo.txt]],
[[/Applications/App.app/Contents/MacOS/app_executable]],
[[/Volumes/ExternalDrive/Data/file.xlsx]],
}

assert_tails(paths)
end)
end
end)

describe("split_lines", function()
local expect = {
"",
"",
"line3 of the file",
"",
"line5 of the file",
"",
"",
"line8 of the file, last line of file",
"",
}

local function get_fake_file(line_ending)
return table.concat(expect, line_ending)
end

local newline_file = get_fake_file "\n"
local carriage_newline_file = get_fake_file "\r\n"

if utils.iswin then
describe("handles files on Windows", function()
it("reads file with newline only", function()
assert.are.same(expect, utils.split_lines(newline_file))
end)
it("reads file with carriage return and newline", function()
assert.are.same(expect, utils.split_lines(carriage_newline_file))
end)
end)
else
describe("handles files on non Windows environment", function()
it("reads file with newline only", function()
assert.are.same(expect, utils.split_lines(newline_file))
end)
end)
end
end)

0 comments on commit 349660c

Please sign in to comment.