Intro
I am a developer, full of passion to open-source community.
In a long time I spent most of my time working or hanging around. But recently, I sent several small code PRs to neovim and golang/tools that made me felt so great.
So I renewed my blog using mdbook with catpuccin theme, to write down what I have done or working on in my life.
To Find Me
You can easily find me by searching acehinnnqru in social medias, including,
Open Source
This directory would record my what I have done or learnt in open-source community.
I would orgnize it by projects.
Neovim
To project Neovim
PR: fix opts.title behaviors of _make_floating_pop_size
How I found it
Hanging around in issues.
How to solve it
As the issue saying, we need to find out what the _make_floating_pop_size does. After travelling the codebase, I found that the opts.title works well as string and I don't know why it should be [string,string][].
After talking with the issue author, I knew that the vim.lsp.util.open_floating_preview using vim.api.neovim_open_win underhood. So it should follow the parameters definitions.
So, it's so easy to solve that _make_floating_pop_size should consider opts.title could be [string,string][].
Solve it
I wrote the first version code and sent it.
if opts.title then
local title = opts.title
local title_length = 0
if type(title) == 'string' then
title_length = vim.fn.strdisplaywidth(title)
else
if title ~= nil then
for i = 1, #title do
title_length = title_length + string.len(title[i][1])
end
end
end
width = math.max(width, title_length)
This version used the wrong api string.len which calculates the wrong length of cjk characters. Used vim.fn.strdisplaywidth instead.
On the another hand, this implementation of this piece is not so elegant.
The @luukvbaal recommends a better version,
local title_length = 0
local chunks = type(opts.title) == 'string' and { { opts.title } } or opts.title or {}
for _, chunk in
ipairs(chunks --[=[@as [string, string][]]=])
do
title_length = title_length + vim.fn.strdisplaywidth(chunk[1])
It's more elegant! I didn't know the --[=[@as [string, string][]]=] syntax of lua, and it really impressed me.
Before requesting code review, I wrote a simple unittest following the old unittests,
it('considers [string,string][] title when computing width', function()
eq(
{ 17, 2 },
exec_lua(function()
return {
vim.lsp.util._make_floating_popup_size(
{ 'foo', 'bar' },
{ title = { { 'A very ', 'Normal' }, { 'long title', 'Normal' } } }
),
}
end)
)
end)
Then, pushed it and requested code review.
After a while, @zeertzjq merged it.
What I learnt
- It's no so hard to finish a PR to a big codebase that it look like.
PR: set correct '] mark when pasting chunks
How I found it
Hanging around in issues.
How to solve it
This issue was solved with several lines' change, but I had changed the implementation many times that I pushed about 20+ commits.
When I first pushed some code, @zeertzjq reviewed and comment some changes. I changed several times between two implementations which differed in the if branches.
After I finalized changes, @zeertzjq invited @justinmk to review. @justinmk rewised me to change some logic which is the same as the first version implementation.
I was changing between several implementations again and again.
Then I realized that I made a mistake that I didn't write a summary for the logic to make all the reviewers to know why I impl it like this. That's why when different reviewers would argued between those changes before.
Here is the merged version,
--- a/runtime/lua/vim/_editor.lua
+++ b/runtime/lua/vim/_editor.lua
@@ -213,7 +213,7 @@ end
vim.inspect = vim.inspect
do
- local tdots, tick, got_line1, undo_started, trailing_nl = 0, 0, false, false, false
+ local startpos, tdots, tick, got_line1, undo_started, trailing_nl = nil, 0, 0, false, false, false
--- Paste handler, invoked by |nvim_paste()|.
---
@@ -328,7 +328,13 @@ do
-- message when there are zero dots.
vim.api.nvim_command(('echo "%s"'):format(dots))
end
+ if startpos == nil then
+ startpos = vim.fn.getpos("'[")
+ else
+ vim.fn.setpos("'[", startpos)
+ end
if is_last_chunk then
+ startpos = nil
vim.api.nvim_command('redraw' .. (tick > 1 and '|echo ""' or ''))
end
return true -- Paste will not continue if not returning `true`.
What I learnt
- You should summarize why you write it like this when there got several different implementations.
- If you think you are right, you should make a brief and clear summary on your code.
Golang
To project Golang or sub projects under it.
Tools
To project Golang Tools
PR: preserves comments in mapsloop
How I found it
Hanging around in issues.
How to solve it
The solution is quite straight as the issue described. We can simplely collects the comments using allComments and put the comments before the target line.
As the allComments function is inside minmax, we ought to move it out. (Thanks for Alan Donovan reviewed and recommonded moved it out instead of simplely copied it.)
At first, I just collected all the comments and put it to start node no matter mrhs is nil or not. But it behaved wrong when mrhs != nil.
When mrhs != nil, the start node is located as the arrow indicates,
m := make(map[string]string)
^
So we would add the comments at wrong position, to correct it, we should used curPrev.Node(), which is the node before rng , instead of rng node.
So the final changes is,
--- a/gopls/internal/analysis/modernize/maps.go
+++ b/gopls/internal/analysis/modernize/maps.go
@@ -156,16 +156,35 @@ func mapsloop(pass *analysis.Pass) {
start, end token.Pos
)
if mrhs != nil {
- // Replace RHS of preceding m=... assignment (and loop) with expression.
- start, end = mrhs.Pos(), rng.End()
- newText = fmt.Appendf(nil, "%s%s(%s)",
+ // Replace assignment and loop with expression.
+ //
+ // m = make(...)
+ // for k, v := range x { /* comments */ m[k] = v }
+ //
+ // ->
+ //
+ // /* comments */
+ // m = maps.Copy(x)
+ curPrev, _ := curRange.PrevSibling()
+ start, end = curPrev.Node().Pos(), rng.End()
+ newText = fmt.Appendf(nil, "%s%s = %s%s(%s)",
+ allComments(file, start, end),
+ analysisinternal.Format(pass.Fset, m),
prefix,
funcName,
analysisinternal.Format(pass.Fset, x))
} else {
// Replace loop with call statement.
+ //
+ // for k, v := range x { /* comments */ m[k] = v }
+ //
+ // ->
+ //
+ // /* comments */
+ // maps.Copy(m, x)
start, end = rng.Pos(), rng.End()
- newText = fmt.Appendf(nil, "%s%s(%s, %s)",
+ newText = fmt.Appendf(nil, "%s%s%s(%s, %s)",
+ allComments(file, start, end),
prefix,
funcName,
analysisinternal.Format(pass.Fset, m),
What I learnt
- Never repeat yourself.