Incorrect `fix-indent` behaviour when writing for loops at root scope

tomlau10 commented 1 month ago

How are you using the lua-language-server?

Visual Studio Code Extension (sumneko.lua)

Which OS are you using?

MacOS

What is the issue affecting?

Formatting

Expected Behaviour

The next line should be indented with the following code

example 1
```
for i = 1, 10 do<enter>
```
example 2
```
do
for i = 1, 10 do<enter>
```

Actual Behaviour

The next line indented correctly initially (according to the indentationRules.increaseIndentPattern), but then it get removed by luals's "auto-fix-indent"...

Reproduction steps

create a new file
write the code snippet provided above
then press enter and observe the behaviour

Additional Notes

I disabled the fix-indent by modifying luals code locally, and this bug stopped. So it's related to the fix-indent function added recently.

I add a do return end here to disable it: https://github.com/LuaLS/lua-language-server/blob/01c6b6136da46573a4c2a1e581e35ca21d4f9b1e/script/core/fix-indent.lua#L194-L195

As discussed in here https://github.com/LuaLS/lua-language-server/discussions/2786, these new indentation behaviours are somewhat annoying and may not be applicable for all of us. Can this be made as optional please 🥲 ? Say an option which can be turned on/off.

Log File

No response

tomlau10 commented 1 month ago

我剛 checkout 了最新 3.10.4，發現 example 1 的問題仍然存在 😕 @sumneko (example 2 則是解決了 👏 )

具體是在文件的 line 1 就輸入 for i = 1, 10 do<enter>，這時縮排會被 auto-fix 掉。
```
1 for i = 1, 10 do<enter>
2 |<-- wrong
```

但若先隔1個空行，則正確

1
2 for i = 1, 10 do<enter>
3     |<-- ok

稍為 debug 了下是 fixWrongIndent() 觸發的然後加了些 debug print log print(myBlock.bstart, lastOffset)
- 出問題的 case (forloop 寫在 line1) 顯示為 14 16
- 沒問題的 case (forloop 寫在 line2) 顯示為 10014 18

我不知道是什麼 logic 出了問題? 是 lastOffset 計算錯誤? 🙄

是否需要開新 issue 來 track? (這 issue 好像沒法 reopen 了)

另外這個 fixWrongIndent 是為了解決什麼問題的呀 😅 我在 discussion 那邊問了 @CppCXY 也沒有得到回覆。。。https://github.com/LuaLS/lua-language-server/discussions/2786#discussioncomment-10253007

sumneko commented 1 month ago

为了解决 local s = 'function'

tomlau10 commented 1 month ago

为了解决 local s = 'function'

我之前研究過可以用 vscode 的 increaseIndentPattern regex 處理 (雖然該 regex 是有些複雜) 簡單來說，該 regex 是由行首起，逐個 character 做 match，如果遇到 ' 或 "，則 跳過整個 string (然後繼續 match 下去)

const stage6 = /^((?!\-\-)[^"']|"[^"\\]*(\\.[^"\\]*)*"|'[^'\\]*(\\.[^'\\]*)*')*(\b(else|function|then|do|repeat)\b((?<=function)[^)]+\)?)?|[\{\(])\s*(\-\-.*)?$/;

我在 vscode 開了1個 issue: https://github.com/microsoft/vscode/issues/199223#issuecomment-2247951065, 裡邊也有測試代碼

但過程中發現 另1個 vscode 的問題。。。 vscode 會將 line 中的 string / comment 內容 先移除掉 bracket (指 ( ) { } [ ]) 由於 lua 的 block quote string / comment 是用 [[ ]] / --[[ ]] 這就導致此特殊情況不可能用 regex 來解決，因為 if s --[[comment]] then 實際上被 vscode 換成 if s --comment then 然後才會調用 increaseIndentPattern

我也開了另1個 issue 來 track 這個: https://github.com/microsoft/vscode/issues/223606 並提議 vscode 是把 inline 的 string / comment 完全去掉 (變成單個 space)，然後才 pass 給 increaseIndentPattern 這樣上 if s --[[comment]] then 可以換成 if s then => 可以如常用 increaseIndentPattern 處理但完全沒得到任何關注 🤦‍♂️

sumneko commented 1 month ago

试试：

local s = 'abc\z
function'

tomlau10 commented 1 month ago

分行後的 string 確實不行。。。因為 vscode 那個 regex 只能 match 單行但是我的 regex 還可以修改

在 match 出 else|then|do|repeat keyword 後，我只允許後邊是 \s*(--.*)? 換句話以下這個 case 我的 regex 是能 handle 的

local s = 'abc\z
else'

但是 function 較特殊，因為 function 後邊可以有 arg list 我用最簡單的 (?<=function)[^)]+\)? 來 match 了這裡可以優化一下，只允許 [\w\s,] 而不是 [^)] 應該也能解決你的 function' 🤔

tomlau10 commented 1 month ago

當然也有 case 肯定不能用 regex 處理，比如是 block comment 內

--[[
this is a function
    |<--這個用 vscode 的 regex 機制肯定解決不了
]]

我當時有查看過 vscode auto indent 的相關 issue 有人是提出過，如果當前是在 comment / string 內，就不觸發 increaseIndentPattern 但好像一直沒有 implement 出來。。。

CppCXY commented 1 month ago

这些方案都不如让语言服务自己决定

CppCXY commented 1 month ago

vscode 基于正则的grammar和相关规则只是足够简单, 但并不是足够好用, 目前语言服务上能初步控制缩进的方式就是typeformat, 但他并不能很好的控制光标位置, 以及由于语法不完整导致的错误计算, 所以还得看未来LSP有没有什么新规范

tomlau10 commented 1 month ago

这些方案都不如让语言服务自己决定

問一下，目前 luals 好像只能將 false positive 的 indent 做移除嗎? 我試了一下

if true --[[comment]] then

這句應該需要 indent
但因為 vscode 內建的 lua increaseIndentPattern 目前寫法是會忽略 -- 以後的內容 => vscode 那邊不會 indent => false negative
然後 luals 這邊好像也不會增加 indent 🤔

CppCXY commented 1 month ago

这些方案都不如让语言服务自己决定

問一下，目前 luals 好像只能將 false positive 的 indent 做移除嗎? 我試了一下
if true --[[comment]] then
這句應該需要 indent

但因為 vscode 內建的 lua increaseIndentPattern 目前寫法是會忽略 -- 以後的內容 => vscode 那邊不會 indent => false negative

然後 luals 這邊好像也不會增加 indent 🤔

不知道, 我不懂, 你问sumneko吧

sumneko commented 1 month ago

目前只做移除，少了缩进补回来比较简单。

tomlau10 commented 1 month ago

如果 luals 是打算 自行做 indent ~或許在終極版本下 (先假設沒有 bug 啦)，直接覆寫 increaseIndentPattern 不做任何 indent~ ~然後每次按 enter 後，都由 luals 自行補上 indent ?~

~這樣就不需要考慮 做移除 🤔~

edit: 好像不行，這個想法有些危險。。。我看 vscode 文檔 increaseIndentPattern 會用於 paste / move line https://code.visualstudio.com/api/language-extensions/language-configuration-guide#indentation-rules

indentationRules defines how the editor should adjust the indentation of current line or next line when you type, paste, and move lines.

如果沒了 increaseIndentPattern 似會爆炸 💥🙈

sumneko commented 1 month ago

服务器延迟比较高，缩进还是挺需要响应速度的，所以服务器顶多修补一下。

tomlau10 commented 1 month ago

我前邊提到 vscode 目前是有一個邏輯，預先將 brackets 從 line 中的 string / comment 移除，然後才交給 increaseIndentPattern: https://github.com/microsoft/vscode/blob/main/src/vs/editor/common/languages/supports/indentationLineProcessor.ts#L214-L222

        tokens.forEach((tokenIndex: number) => {
            const tokenType = tokens.getStandardTokenType(tokenIndex);
            let text = tokens.getTokenText(tokenIndex);
            if (shouldRemoveBracketsFromTokenType(tokenType)) {
                text = text.replace(bracketsRegExp, '');
            }
            const metadata = tokens.getMetadata(tokenIndex);
            textAndMetadata.push({ text, metadata });
        });

這個是因為大部分語言都是用 bracket 來定義 scope / block 而先從 string / comment 把 bracket 移除掉，就不會令該些語言的 increaseIndentPattern 出現誤判
遺憾的是 lua 是 keyword based... 以 keyword 來決定 scope，所以單純移除 bracket 沒有相應效果

忽發奇想

既然目前 vscode 會將 language-configuration.json 中的 brackets 移除掉那我就將 keyword pairs 也寫進 brackets https://github.com/LuaLS/lua-language-server/blob/d702a55715df19a219e963da496e6fb76db0aacd/script/provider/language-configuration.lua#L17-L19

local languageConfiguration = {
    id = 'lua',
    configuration = {
        brackets = {
            { "{", "}" },
            { "[", "]" },
            { "(", ")" },
            { "function", "end" },
            { "then", "end" },
            { "else", "end" },
            { "do", "end" },
            { "repeat", "until" },
        },

然後關掉 luals 的 fix-indent 來測試 => 果然有效 😂 上邊的 multi line string / block comment 也不會出現 false positive 縮排了說明 vscode 的 先將 line 中 comment / string 移除掉會影響 indent 的字符 方向是可行的
但是將 keyword 放到 brackets 完全不現實 ❌
- 會被當成 brackets，highlight 顏色就錯誤了
- 並且 keywords 不一定是 matching pair，比如 if then else end 結構，then & else 當成 open，卻只有1個 end close bracket => unmatch

所以如果說 vscode 的 language-configuration.json 允許定義一個 keywords[] 之類

是會在 process line 時，對 string / comment 做移除的然後對於 lua 我們填入 ["else", "function", "then", "do", "repeat", "end", "until"] (只需填 increaseIndentPattern 用到的) 這樣應該就能徹底地解決問題 🤔
而有了這個邏輯後 increaseIndentPattern 也不需考慮 comment -- 問題，只需考慮 keywords 因為假如 keywords 出現在 string / comment，肯定已經被移除掉就是說 if true --[[then]] then 這種 case => 這會被換成 if true -- then 但並不需要考慮 then 是否在 -- 後邊，因為沒被移除的肯定都不是位置 comment / string 之內
- increaseIndentPattern 單純用最初的 ((\\b(else|function|then|do|repeat)\\b((?!\\b(end|until)\\b).)*)|(\\{\\s*))$ 就可以

(但是要推動 vscode 的改動，好像難於登天。。。)

LuaLS / lua-language-server