Skip to content

Commit a76bf27

Browse files
aweis89aweis89
committed
refactor(references): simplify to only support file:// URIs
Remove legacy path:line and path-only pattern detection in favor of exclusively parsing file:// URIs. This simplifies the code significantly and avoids false positives from ambiguous path patterns. The system prompt instructs the LLM to use file:// URIs, making the legacy patterns unnecessary.
1 parent 4fee569 commit a76bf27

File tree

5 files changed

+30
-183
lines changed

5 files changed

+30
-183
lines changed

‎lua/opencode/ui/reference_picker.lua‎

Lines changed: 26 additions & 179 deletions
Original file line numberDiff line numberDiff line change
@@ -1,93 +1,11 @@
1-
-- Code reference picker for navigating to file:line references in LLM responses
1+
-- Code reference picker for navigating to file:// URI references in LLM responses
22
local state = require('opencode.state')
33
local config = require('opencode.config')
44
local base_picker = require('opencode.ui.base_picker')
55
local icons = require('opencode.ui.icons')
66

77
local M = {}
88

9-
-- File extensions to recognize as valid code files
10-
local VALID_EXTENSIONS = {
11-
'lua',
12-
'py',
13-
'js',
14-
'ts',
15-
'tsx',
16-
'jsx',
17-
'go',
18-
'rs',
19-
'c',
20-
'cpp',
21-
'h',
22-
'hpp',
23-
'java',
24-
'rb',
25-
'php',
26-
'swift',
27-
'kt',
28-
'scala',
29-
'sh',
30-
'bash',
31-
'zsh',
32-
'json',
33-
'yaml',
34-
'yml',
35-
'toml',
36-
'xml',
37-
'html',
38-
'css',
39-
'scss',
40-
'md',
41-
'txt',
42-
'vim',
43-
'el',
44-
'ex',
45-
'exs',
46-
'erl',
47-
'hs',
48-
'ml',
49-
'fs',
50-
'clj',
51-
'r',
52-
'sql',
53-
'graphql',
54-
'proto',
55-
'tf',
56-
'nix',
57-
'zig',
58-
'v',
59-
'svelte',
60-
'vue',
61-
}
62-
63-
-- Build a lookup set for faster extension checking
64-
local EXTENSION_SET = {}
65-
for _, ext in ipairs(VALID_EXTENSIONS) do
66-
EXTENSION_SET[ext] = true
67-
end
68-
69-
---Check if a file extension is valid
70-
---@param ext string
71-
---@return boolean
72-
local function is_valid_extension(ext)
73-
return EXTENSION_SET[ext:lower()] == true
74-
end
75-
76-
---Check if the path looks like a URL (to avoid false positives)
77-
---@param text string
78-
---@param match_start number
79-
---@return boolean
80-
local function is_url_context(text, match_start)
81-
-- Check if preceded by :// (like http://, file://, etc.)
82-
if match_start > 3 then
83-
local prefix = text:sub(match_start - 3, match_start - 1)
84-
if prefix:match('://$') then
85-
return true
86-
end
87-
end
88-
return false
89-
end
90-
919
---Check if a file exists
9210
---@param file_path string
9311
---@return boolean
@@ -110,120 +28,49 @@ end
11028
---@field file string Absolute file path (for Snacks picker preview)
11129
---@field pos number[]|nil Position as {line, col} for Snacks picker preview
11230

113-
---Parse references from text
31+
---Parse file:// URI references from text
11432
---@param text string The text to parse
11533
---@param message_id string The message ID for tracking
11634
---@return CodeReference[]
11735
function M.parse_references(text, message_id)
11836
local references = {}
119-
local covered_ranges = {} -- Track which character ranges we've already matched
120-
121-
-- Helper to check if a range overlaps with any covered range
122-
local function is_covered(start_pos, end_pos)
123-
for _, range in ipairs(covered_ranges) do
124-
-- Check if ranges overlap
125-
if not (end_pos < range[1] or start_pos > range[2]) then
126-
return true
127-
end
128-
end
129-
return false
130-
end
131-
132-
-- Helper to add a reference
133-
local function add_reference(path, ext, match_start, match_end, line, column)
134-
if not is_valid_extension(ext) then
135-
return false
136-
end
137-
if not file_exists(path) then
138-
return false
139-
end
140-
if is_covered(match_start, match_end) then
141-
return false
142-
end
143-
144-
-- Mark this range as covered
145-
table.insert(covered_ranges, { match_start, match_end })
146-
147-
-- Create absolute path for Snacks preview
148-
local abs_path = path
149-
if not vim.startswith(path, '/') then
150-
abs_path = vim.fn.getcwd() .. '/' .. path
151-
end
15237

153-
table.insert(references, {
154-
file_path = path,
155-
line = line,
156-
column = column,
157-
message_id = message_id,
158-
match_start = match_start,
159-
match_end = match_end,
160-
file = abs_path,
161-
pos = line and { line, (column or 1) - 1 } or nil,
162-
})
163-
return true
164-
end
165-
166-
-- First pass: find file:// URI references (preferred format)
167-
-- Matches: file://path/to/file.ext or file://path/to/file.ext:line or file://path/to/file.ext:line:column
168-
local pattern_file_uri = 'file://([%w_./%-]+%.([%w]+)):?(%d*):?(%d*)'
38+
-- Match file:// URIs with optional line and column numbers
39+
-- Formats: file://path/to/file or file://path/to/file:line or file://path/to/file:line:column
40+
local pattern = 'file://([%w_./%-]+):?(%d*):?(%d*)'
16941
local search_start = 1
170-
while search_start <= #text do
171-
local match_start, match_end, path, ext, line_str, col_str = text:find(pattern_file_uri, search_start)
172-
if not match_start then
173-
break
174-
end
17542

176-
local line = line_str ~= '' and tonumber(line_str) or nil
177-
local column = col_str ~= '' and tonumber(col_str) or nil
178-
add_reference(path, ext, match_start, match_end, line, column)
179-
search_start = match_end + 1
180-
end
181-
182-
-- Second pass: find path:line[:column] references (legacy format, more specific)
183-
local pattern_with_line = '([%w_./%-]+%.([%w]+)):(%d+):?(%d*)'
184-
search_start = 1
18543
while search_start <= #text do
186-
local match_start, match_end, path, ext, line_str, col_str = text:find(pattern_with_line, search_start)
44+
local match_start, match_end, path, line_str, col_str = text:find(pattern, search_start)
18745
if not match_start then
18846
break
18947
end
19048

191-
-- Skip if this looks like a URL (http://, https://, file://, etc.)
192-
if is_url_context(text, match_start) then
193-
search_start = match_end + 1
194-
else
195-
local line = tonumber(line_str)
49+
-- Only add if file exists
50+
if file_exists(path) then
51+
local line = line_str ~= '' and tonumber(line_str) or nil
19652
local column = col_str ~= '' and tonumber(col_str) or nil
197-
add_reference(path, ext, match_start, match_end, line, column)
198-
search_start = match_end + 1
199-
end
200-
end
20153

202-
-- Third pass: find path-only references (must contain a slash to be a path)
203-
local pattern_no_line = '([%w_%-]+/[%w_./%-]+%.([%w]+))'
204-
search_start = 1
205-
while search_start <= #text do
206-
local match_start, match_end, path, ext = text:find(pattern_no_line, search_start)
207-
if not match_start then
208-
break
209-
end
54+
-- Create absolute path for Snacks preview
55+
local abs_path = path
56+
if not vim.startswith(path, '/') then
57+
abs_path = vim.fn.getcwd() .. '/' .. path
58+
end
21059

211-
-- Skip if preceded by file:// or other URL scheme
212-
if is_url_context(text, match_start) then
213-
search_start = match_end + 1
214-
-- Only add if not followed by a colon and digit (which would be caught by second pattern)
215-
elseif text:sub(match_end + 1, match_end + 1) ~= ':' then
216-
add_reference(path, ext, match_start, match_end, nil, nil)
217-
search_start = match_end + 1
218-
else
219-
search_start = match_end + 1
60+
table.insert(references, {
61+
file_path = path,
62+
line = line,
63+
column = column,
64+
message_id = message_id,
65+
match_start = match_start,
66+
match_end = match_end,
67+
file = abs_path,
68+
pos = line and { line, (column or 1) - 1 } or nil,
69+
})
22070
end
221-
end
22271

223-
-- Sort by match position for consistent ordering
224-
table.sort(references, function(a, b)
225-
return a.match_start < b.match_start
226-
end)
72+
search_start = match_end + 1
73+
end
22774

22875
return references
22976
end

‎tests/data/cursor_data.expected.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"extmarks":[[1,1,0,{"ns_id":3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":false,"virt_text":[["▌󰭻 ","OpencodeMessageRoleUser"],[" "],["USER","OpencodeMessageRoleUser"],["","OpencodeHint"],[" (2025-10-30 17:42:54)","OpencodeHint"],[" [msg_a3637244a001FDRDfoBYVPEGpd]","OpencodeHint"]],"virt_text_pos":"win_col","virt_text_win_col":-3,"priority":10}],[2,2,0,{"ns_id":3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text":[["▌","OpencodeMessageRoleUser"]],"virt_text_pos":"win_col","virt_text_win_col":-3,"priority":4096}],[3,3,0,{"ns_id":3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text":[["▌","OpencodeMessageRoleUser"]],"virt_text_pos":"win_col","virt_text_win_col":-3,"priority":4096}],[4,4,0,{"ns_id":3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text":[["▌","OpencodeMessageRoleUser"]],"virt_text_pos":"win_col","virt_text_win_col":-3,"priority":4096}],[5,5,0,{"ns_id":3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text":[["▌","OpencodeMessageRoleUser"]],"virt_text_pos":"win_col","virt_text_win_col":-3,"priority":4096}],[6,6,0,{"ns_id":3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text":[["▌","OpencodeMessageRoleUser"]],"virt_text_pos":"win_col","virt_text_win_col":-3,"priority":4096}],[7,7,0,{"ns_id":3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text":[["▌","OpencodeMessageRoleUser"]],"virt_text_pos":"win_col","virt_text_win_col":-3,"priority":4096}],[8,8,0,{"ns_id":3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text":[["▌","OpencodeMessageRoleUser"]],"virt_text_pos":"win_col","virt_text_win_col":-3,"priority":4096}],[9,9,0,{"ns_id":3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text":[["▌","OpencodeMessageRoleUser"]],"virt_text_pos":"win_col","virt_text_win_col":-3,"priority":4096}],[10,10,0,{"ns_id":3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text":[["▌","OpencodeMessageRoleUser"]],"virt_text_pos":"win_col","virt_text_win_col":-3,"priority":4096}],[11,11,0,{"ns_id":3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text":[["▌","OpencodeMessageRoleUser"]],"virt_text_pos":"win_col","virt_text_win_col":-3,"priority":4096}],[12,13,0,{"ns_id":3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":false,"virt_text":[[" ","OpencodeMessageRoleAssistant"],[" "],["BUILD","OpencodeMessageRoleAssistant"],[" gpt-5-mini","OpencodeHint"],[" (2025-10-30 17:42:56)","OpencodeHint"],[" [msg_a36372b9a001M1lQEix4SK5QE5]","OpencodeHint"]],"virt_text_pos":"win_col","virt_text_win_col":-3,"priority":10}]],"timestamp":1766369775,"actions":[],"lines":["----","","","what is this doing","","[lua/opencode/context.lua](lua/opencode/context.lua)","","Line 59:","`````lua","local is_enabled = vim.tbl_get(config, 'context', context_key, 'enabled')","`````","","----","","","**Explanation**","","- The line `local is_enabled = vim.tbl_get(config, 'context', context_key, 'enabled')` (in ` lua/opencode/context.lua:58`) uses `vim.tbl_get` to safely read a nested field from a table.","- Concretely it attempts to read `config.context[context_key].enabled` but without throwing an error if `config.context` or `config.context[context_key]` is nil. If any intermediate key is missing it returns `nil`.","- In the surrounding function `M.is_context_enabled`, that value is the default config value for the given context key. The function then checks the state override:"," - If `state.current_context_config[context_key].enabled` is not `nil`, that state value (true/false) is returned."," - Otherwise the `is_enabled` value (from `config`) is returned.","- Example: if `config.context.selection.enabled == true` but `state.current_context_config.selection.enabled == false`, the function returns `false` (state overrides config). If the state value is `nil`, the config value is used.","- Why this matters: `vim.tbl_get` provides safe nested access; using it avoids runtime errors when some parts of the nested config are absent.","",""]}
1+
{"actions":[],"extmarks":[[1,1,0,{"virt_text":[["▌󰭻 ","OpencodeMessageRoleUser"],[" "],["USER","OpencodeMessageRoleUser"],["","OpencodeHint"],[" (2025-10-30 17:42:54)","OpencodeHint"],[" [msg_a3637244a001FDRDfoBYVPEGpd]","OpencodeHint"]],"ns_id":3,"virt_text_win_col":-3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":false,"virt_text_pos":"win_col","priority":10}],[2,2,0,{"virt_text":[["▌","OpencodeMessageRoleUser"]],"ns_id":3,"virt_text_win_col":-3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text_pos":"win_col","priority":4096}],[3,3,0,{"virt_text":[["▌","OpencodeMessageRoleUser"]],"ns_id":3,"virt_text_win_col":-3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text_pos":"win_col","priority":4096}],[4,4,0,{"virt_text":[["▌","OpencodeMessageRoleUser"]],"ns_id":3,"virt_text_win_col":-3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text_pos":"win_col","priority":4096}],[5,5,0,{"virt_text":[["▌","OpencodeMessageRoleUser"]],"ns_id":3,"virt_text_win_col":-3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text_pos":"win_col","priority":4096}],[6,6,0,{"virt_text":[["▌","OpencodeMessageRoleUser"]],"ns_id":3,"virt_text_win_col":-3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text_pos":"win_col","priority":4096}],[7,7,0,{"virt_text":[["▌","OpencodeMessageRoleUser"]],"ns_id":3,"virt_text_win_col":-3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text_pos":"win_col","priority":4096}],[8,8,0,{"virt_text":[["▌","OpencodeMessageRoleUser"]],"ns_id":3,"virt_text_win_col":-3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text_pos":"win_col","priority":4096}],[9,9,0,{"virt_text":[["▌","OpencodeMessageRoleUser"]],"ns_id":3,"virt_text_win_col":-3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text_pos":"win_col","priority":4096}],[10,10,0,{"virt_text":[["▌","OpencodeMessageRoleUser"]],"ns_id":3,"virt_text_win_col":-3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text_pos":"win_col","priority":4096}],[11,11,0,{"virt_text":[["▌","OpencodeMessageRoleUser"]],"ns_id":3,"virt_text_win_col":-3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":true,"virt_text_pos":"win_col","priority":4096}],[12,13,0,{"virt_text":[[" ","OpencodeMessageRoleAssistant"],[" "],["BUILD","OpencodeMessageRoleAssistant"],[" gpt-5-mini","OpencodeHint"],[" (2025-10-30 17:42:56)","OpencodeHint"],[" [msg_a36372b9a001M1lQEix4SK5QE5]","OpencodeHint"]],"ns_id":3,"virt_text_win_col":-3,"virt_text_hide":false,"right_gravity":true,"virt_text_repeat_linebreak":false,"virt_text_pos":"win_col","priority":10}]],"timestamp":1766370765,"lines":["----","","","what is this doing","","[lua/opencode/context.lua](lua/opencode/context.lua)","","Line 59:","`````lua","local is_enabled = vim.tbl_get(config, 'context', context_key, 'enabled')","`````","","----","","","**Explanation**","","- The line `local is_enabled = vim.tbl_get(config, 'context', context_key, 'enabled')` (in `lua/opencode/context.lua:58`) uses `vim.tbl_get` to safely read a nested field from a table.","- Concretely it attempts to read `config.context[context_key].enabled` but without throwing an error if `config.context` or `config.context[context_key]` is nil. If any intermediate key is missing it returns `nil`.","- In the surrounding function `M.is_context_enabled`, that value is the default config value for the given context key. The function then checks the state override:"," - If `state.current_context_config[context_key].enabled` is not `nil`, that state value (true/false) is returned."," - Otherwise the `is_enabled` value (from `config`) is returned.","- Example: if `config.context.selection.enabled == true` but `state.current_context_config.selection.enabled == false`, the function returns `false` (state overrides config). If the state value is `nil`, the config value is used.","- Why this matters: `vim.tbl_get` provides safe nested access; using it avoids runtime errors when some parts of the nested config are absent.","",""]}

‎tests/data/diagnostics.expected.json‎

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)