| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
Notice: It looks like you're using hustcer/deepseek-review, but the CHAT_TOKEN hasn't been configured in your repo's Variables/Secrets. Please ensure this token is set for proper functionality. For step-by-step guidance, refer to the CHAT_TOKEN Config section of README.
Sorry, something went wrong.
@copilot Please review it in Chinese |
Sorry, something went wrong.
|
@github-actions review一下 |
Sorry, something went wrong.
There was a problem hiding this comment.
中文:整体方向正确:用 util.viewLiteral/util.escapeInvalidUtf8 统一处理非法 UTF-8,并在 hover 截断时按字符边界切割,避免产生无效 UTF-8。但有几处需要确认或修正。
local nextCharacter = utf8.offset(view, 0, max + 1)
view = view:sub(1, nextCharacter - 1) .. '...'utf8.offset(s, 0, i) 返回的是包含第 i 个字节的字符的起始位置,而不是“下一个字符”的位置。当 max + 1 恰好落在一个字符的起始字节上时,nextCharacter 就等于 max + 1,于是 sub(1, max) 会把这个字符截掉一半,仍然可能产生无效 UTF-8;当 max + 1 落在字符中间时,返回的是该字符的起始位置,sub(1, nextCharacter - 1) 又会比预期少截一个字符。建议改为先取 utf8.offset(view, max + 1)(即第 max + 1 个字符的起始位置),再 sub(1, nextCharacter - 1);或直接用 utf8.offset(view, max) 得到第 max 个字符的结束位置。另外 utf8.offset 在越界时会返回 nil,虽然此处 #view > max 保证不会越界,但建议加断言或兜底,避免后续改动引入崩溃。
function m.escapeInvalidUtf8(str)
if utf8Len(str) then
return str
end
...viewString 中原本是 if not utf8Len(str) then str = escapeInvalidUtf8(str) end,现在等价,没问题。但 m.escapeInvalidUtf8 现在是公开 API,返回的可能是入参本身(同一引用)。如果调用方会修改返回值,可能意外修改原字符串。建议在文档/注释中说明,或统一返回副本。
util.viewLiteral 的行为需要确认:('%q'):format(key) 与 viewLiteral 的输出是否在所有情况下一致?测试里期望 '"\\194"',说明 viewLiteral 对非法字节做了转义。但 %q 对 \0、\n、\r 等的转义规则与 viewLiteral 是否一致?如果 viewLiteral 使用了 viewString 的 [[...]] 长字符串形式,那么对于包含 ]] 或换行的 key,输出格式会与原来的 %q 不同,可能影响 completion label 的展示或客户端解析。建议补充针对 \n、\0、]] 等边界 key 的测试。
测试断言 assert(utf8.len(value)) 和 assert(utf8.len(json.encode {...})) 只验证了“不是 nil”,即字符串是合法 UTF-8。这符合本次修复目标,但 utf8.len 对空字符串返回 0(真值),对非法 UTF-8 返回 nil, pos。建议在断言失败时输出位置信息,便于定位,例如 assert(utf8.len(value), ('invalid utf8 at %d'):format(select(2, utf8.len(value))))。
changelog.md 描述准确,但建议同时说明 hover 截断现在按字符边界处理,避免用户误以为只修了 completion。
English: The overall direction is correct: unifying invalid UTF-8 handling via util.viewLiteral/util.escapeInvalidUtf8 and truncating hover output on character boundaries avoids producing invalid UTF-8. However, several points need confirmation or fixes.
local nextCharacter = utf8.offset(view, 0, max + 1)
view = view:sub(1, nextCharacter - 1) .. '...'utf8.offset(s, 0, i) returns the start position of the character containing byte i, not the position of the "next" character. When max + 1 happens to be the first byte of a character, nextCharacter equals max + 1, so sub(1, max) cuts that character in half and can still yield invalid UTF-8. When max + 1 falls inside a character, it returns that character's start, so sub(1, nextCharacter - 1) truncates one character less than intended. Prefer utf8.offset(view, max + 1) (start of the (max+1)-th character) then sub(1, nextCharacter - 1), or use utf8.offset(view, max) to get the end of the max-th character. Also, utf8.offset returns nil when out of range; although #view > max guards this today, add an assertion or fallback to avoid future crashes.
function m.escapeInvalidUtf8(str)
if utf8Len(str) then
return str
end
...In viewString this is equivalent to the previous if not utf8Len(str) then str = escapeInvalidUtf8(str) end, so it's fine there. But m.escapeInvalidUtf8 is now public API and may return the input by reference. If callers mutate the result, they could unexpectedly mutate the original string. Document this or return a copy consistently.
Confirm util.viewLiteral behavior: is its output identical to ('%q'):format(key) in all cases? The test expects '"\\194"', showing it escapes invalid bytes. But does viewLiteral match %q for \0, \n, \r, etc.? If viewLiteral uses viewString's [[...]] long-string form, keys containing ]] or newlines will render differently from %q, potentially affecting completion label display or client parsing. Add tests for boundary keys like \n, \0, ]].
The test assertions assert(utf8.len(value)) and assert(utf8.len(json.encode {...})) only verify "not nil", i.e. the string is valid UTF-8. That matches the fix's goal, but utf8.len returns 0 (truthy) for empty strings and nil, pos for invalid UTF-8. Include the position in the failure message for easier debugging, e.g. assert(utf8.len(value), ('invalid utf8 at %d'):format(select(2, utf8.len(value)))).
The changelog.md entry is accurate, but also mention that hover truncation now respects character boundaries, so users don't assume only completion was fixed.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
It is possible for lua-language-server to send invalid utf-8.
Some clients (e.g. Zed) will drop the connection if an LSP sends invalid data.
Hovering this will trigger the invalid UTF-8 in the response.
Similarly enum completions and labels for this include raw 0xC2: