Add option to sort discussions by file name

[johnybx]

Hi,
Currently we have discussion tree as list of discussions but when discussion tree is opened it is little bit hard to figure out where the discussions actually are located ( in which files ). You can jump from discussion to reviewer and back but I wanted to have better overview so I was playing around with discussion tree and added option to render file tree which is similar to diffview.nvim file panel. I am still experimenting with it and probably would make sense to write some tests for that 🤔 . Would you agree to add such feature ?

[harrisoncramer]

Yeah I like this idea, the comments by file name looks quite nice.

My only concern is that we are introducing a lot of code to maintain in the discussions module that may be hard to debug, especially since it's all currently untested. I'm not opposed to doing this, but would want to make sure we're being thoughtful about changing this module.

[johnybx]

Hi,
yes, I definitely agree that discussions module is getting quite big. I pushed initial update how we can split discussions and if you agree with this I would further also split signs and diagnostics logic to separate files. I also added scripts to be able to run lua tests for modules which depends on other plugins and neovim specific functions ( for example now you can use vim.inspect in test ). Currently you should be able to run script which will initialize everything for you (if needed) and run tests like this ( dots should be green 😅 ):

❯ ./lua-test.sh
●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●
34 successes / 0 failures / 0 errors / 0 pending : 0.0311 seconds

I want to add more tests for discussions because currently I added just basic tests for initializing unlinked discussion nodes.
We will also need to update github workflow because currently it uses lunarmodules/[email protected] but that is not sufficient for running tests - we will need to update steps so that during tests we have available nvim, luarocks and git commands ( didn't have time to check this yet ).

I am not sure where do you want to document testing and development as such. Have you considered something like https://generator.contributing.md/?

Please let me know if you agree with this approach or if you prefer something else 🙏.

[johnybx]

I updated github workflow and now it runs tests with stable and nightly neovim. I noticed that tests are in 2 workflows - do you want to keep separate lua-tests.yaml or should I move updated workflow to lua.yaml ?

[harrisoncramer]

I updated github workflow and now it runs tests with stable and nightly neovim. I noticed that tests are in 2 workflows - do you want to keep separate lua-tests.yaml or should I move updated workflow to lua.yaml ?

Yeah we should just have a single flow for Lua tests. Thanks for doing this! I'd been planning to set up running tests through Neovim but hadn't figured out how to do it yet.

Let me know when this is functional and ready for review and I'll go through the code.

[johnybx]

I added tests for whole tree.lua ( which was split from discussions ). I wanted to further split discussions/init.lua but I think this PR is pretty big already so it would probably make sense to review it and continue in other PR.

Let me know if there is anything i need to change.

Regarding this question:

I am not sure where do you want to document testing and development as such. Have you considered something like https://generator.contributing.md/?

Did you consider where should be some basic docs for devs ? thanks.

[harrisoncramer]

Hey, thank you for this MR. It's pretty big so I'll try to review it this weekend. And yes I've thought about adding contributing and will work that into my next MR which is also focused mostly on testing.

[harrisoncramer]

I'm getting warnings using Neovim's LSP for all of the NuiTree types, do you know if I have to set up something to get this working?

How do you have your LSP set up to detect these types correctly?

[johnybx]

I've setup neodev like this ( path need to be updated to yours path )

        {
            "folke/neodev.nvim",
            opts = {
                override = function(root_dir, library)
                    if root_dir:match("/workspace/nvim/") then
                        library.enabled = true
                        library.plugins = true
                    end
                end,
                debug = true,
            },
        },

normally neodev loads plugins only for config so you need to force it for other plugins ( there might be other options how to configure this but I found this to be straight forward ).

With current changes for tests you could also setup local config in .luarc.json (ref docs) with paths to local plugins in tests/plugins.

I used these existing classes from plugin because duplicating those to giltab plugin did not seem right.

[harrisoncramer]

Making edits to a note/comment is broken.

Whenever you send a note_id it should be an integer and you are sending a string now. Both the send_deletion and send_edits functions aren't working for me for either the new tree type or the old tree type.

type EditCommentRequest struct {
	Comment      string `json:"comment"`
	NoteId       int    `json:"note_id"`
	DiscussionId string `json:"discussion_id"`
	Resolved     bool   `json:"resolved"`
}

[johnybx]

I fixed and tested create, edit, delete of notes and it works now ( I more often create / resolve comments then edit delete so I didn't notice, sorry ).