Closed jakubbortlik closed 1 month ago
Hey, this is possibly a good idea, but I'd really like to close out your other issues before adding others. There are two other issue requests with your name on them, do you think you'd be able to tackle either of these?
https://github.com/harrisoncramer/gitlab.nvim/issues/282 https://github.com/harrisoncramer/gitlab.nvim/issues/158
You also suggested that you may be able to address this one, I don't know if it's been resolved yet:
Hi, #158 is tricky, but I'll see what I can do about it.
With #247, I think Aravind's point about the "origin/" .. base_branch
being the correct merge target makes sense, so I can prepare a PR with this change.
I'm game to review an MR implementing default keymaps if you wanted to take a stab at it, specifically for the reviewer pane. I think you're right that we shouldn't make people set them up themselves.
It'd have to be configurable, e.g. a "keymaps" object of some sort in settings, where someone can opt into the defaults or not, and then the ability to override each one. We'd also probably make this a breaking change and set them on by default to reduce friction for new users.
@jakubbortlik I'd like to try and get this implemented next to reduce friction for new users, if you want to do it go for it, or I can try, let me know!
Hi @harrisoncramer, I'll try and get a PR ready.
Feature Description
I'd like to share a piece of my
gitlab.nvim
/DiffView
configuration. It creates operator and visual mode mappings in the reviewer windows. Rather than having this in my configuration, I think it would be more suitable to be part ofgitlab.nvim
itself for the following reasons:move_to_discussion_tree_from_diagnostic
only make sense in the reviewer. No need for the mappings to be global.DiffView
reviewer buffers, even those not created bygitlab.nvim
, where they make no sense.vipgls
. With the operator mappings, I can just typesip
, orvips
if I want to first see the visual selection -- since the reviewer windows are non-modifiable,s
andc
are practically free to remap and thecip
,sip
mappings are much easier to type.Using the same keymap for
move_to_discussion_tree_from_diagnostic
as forjump_to_reviewer
it's easier to quickly move between the tree and reviewer without having to think if itsm
orglm
that one has to use.I'd prefer to use
a
rather thanm
for jumping between the reviewer and the tree sincea
is "free" in the non-modifiable buffers, andm
could be left available for setting marks.x
would also be a fine alternative. But of course, I can do this in my personal config, and I can use the existing defaultm
for jumping. The default operator mappings could beglc
andgls
instead ofc
ands
, to be more in line with the current mappings, but I find that unnecessarily conservative.@harrisoncramer, would you be willing to include something like this into the plugin? I imagine this could be called in the
initialize_discussions
function, something like:Where
M.set_reviewer_keymaps
would work similar toM.modifiable
(lua/gitlab/actions/discussions/init.lua:68
).I'd only use
create_multiline_comment
andcreate_comment_suggestion
, sincecreate_comment
is just a special case ofcreate_multiline_comment
anyway, and it can be achieved by simply using a motion that moves on the same line, e.g.,cl
.