Repository navigation
feat: implement rebasing - #534
Conversation
27e544a to
6fc6475
Compare
3384ee1 to
d528de8
Compare
dff2d2a to
49d2495
Compare
| create_mr = "glC", -- Create a new MR for currently checked-out feature branch | ||
| choose_merge_request = "glc", -- Chose MR for review (if necessary check out the feature branch) | ||
| start_review = "glS", -- Start review for the currently checked-out branch | ||
| reload_review = "gl<C-R>", -- Load new MR state from Gitlab and apply new diff refs to the diff view |
There was a problem hiding this comment.
I'm wondering whether it would make sense to connect this to the refresh of the discussion tree instead of making it a separate function and keybinding.
| gitlab.close_review() ~ | ||
|
|
||
| Closes the reviewer tab and discussion tree and cleans up (e.g., removes | ||
| winbar timer). | ||
| >lua | ||
| require("gitlab").close_review() | ||
| < |
There was a problem hiding this comment.
This is an existing API that was just missing documentation.
| M.is_open = true | ||
| local cur_view = diffview_lib.get_current_view() | ||
| M.diffview_layout = cur_view.cur_layout | ||
| M.diffview = require("diffview.lib").get_current_view() |
There was a problem hiding this comment.
Saving this to the module table makes it possible to reduce the reliance on the diffview.lib in other places of the codebase.
a9de870 to
9dd7965
Compare
906e633 to
5aad115
Compare
5aad115 to
567f8a9
Compare
567f8a9 to
4bd4100
Compare
|
Hi @harrisoncramer, do you think you could have a look at this PR? I think it is ready to be merged and find the functionality really useful, and it would unblock me for working on other issues. When I'm working on a fix or new feature, I frequently just test it during my normal work and while doing so I really would like to have the functionality from this PR and also from #552 available. |
291636a to
a231360
Compare
- Rebases the MR on the server. - Pulls the remote branch after rebasing. - Aborts rebase if it is not necessary, but enables forcing a rebase. - Aborts rebase when there are conflicts. - Adds keymaps for common rebase variants, and for reloading reviewer. - Checks if branch is up-to-date asynchronously.
a231360 to
b5368b0
Compare
* feat: Fetch all merge request beyond the default 20 (#549) * feat(choose_merge_request): add per_page option for merge request fetching Allow users to configure the number of merge requests fetched via the `per_page` setting in `choose_merge_request`. Defaults to 100. * Remove the per_page option and now fetch all merge request internally --------- Co-authored-by: Clement Hue <clement.hue@thetrainline.com> * fix: encode empty merge_requests body as JSON object instead of array (#554) When choose_merge_request() is called without arguments we must pass an empty JSON *object* to the Go server instead of an empty *array*. * fix(git): handle detached HEAD silently (#557) Allow reviewing a merge request from detached HEAD. Co-authored-by: Sebastian Flügge <952313+seflue@users.noreply.github.com> * Feature merge request beyond 20 (#551) * feat(choose_merge_request): add per_page option for merge request fetching Allow users to configure the number of merge requests fetched via the `per_page` setting in `choose_merge_request`. Defaults to 100. * Remove the per_page option and now fetch all merge request internally * Implement merge request pagination using gitlab api --------- Co-authored-by: Clement Hue <clement.hue@thetrainline.com> * fix: use correct type annotation for optional parameters (#562) * fix(server): use vim.system instead of jobstart to bypass shell (#560) Switch the server launch from vim.fn.jobstart to vim.system, passing the binary and JSON settings as a list. Neovim spawns the process directly instead of going through the user's shell, removing the Windows-specific "-escaping hack that only worked for POSIX-style shells, otherwise it prevented the Go server from starting. * fix: use light background color for dark labels (#563) Dark MR labels are hard to distinguish from a dark background. Select a light colorscheme-aware background color for such labels, if the vim.o.background is set to dark. * feat: implement rebasing (#534) - Rebases the MR on the server. - Pulls the remote branch after rebasing. - Aborts rebase if it is not necessary, but enables forcing a rebase. - Aborts rebase when there are conflicts. - Adds keymaps for common rebase variants, and for reloading reviewer. - Checks if branch is up-to-date asynchronously. * feat: open discussions immediately load data afterwards (#552) Main features: - opens discussion panel immediately and loads data asynchronously - improves restoring cursor position after rebuilding trees - uses winfixbuf to disable switching to another buffer in discussion panel - discussion panel window options do not leak to reviewer - autocommands are added to clean up state when discussion panel is closed manually Minor improvements: - moves view type switching from winbar to discussions - adds protection from invalid line number - replaces deprecated vim.api function * docs: update references to diffview-plus.nvim (#566) Document that dlyongemallo's fork has been renamed. Also, add a health check that verifies if the recommended (but not required) diffview.api package is present. * docs: add missing dependencies and unify quick start (#567) * refactor: remove dead code related to emojis * feat: pull emoji data from Gitlab Instead of using an incomplete local emojis.json list, the plugin now pulls the emoji data from Gitlab. This makes sure the data is complete and doesn't contain invalid items, which would cause a Go server error (e.g., cmd/config/emojis.json contained ⛵ for ⛵). * fix: only initialize emojis when starting a review * feat: replace plenary Job with vim.system (#556) Plenary is not actively maintained and will be archived soon. Replace plenary.Job with builtin vim.system calls. * refactor: remove unused utils functions * refactor: remove some more unused functions (#571) * fix: only check Go version when building the server (#572) The go version is now only checked when building the server. When the user provides a server binary via configuration, Go version is not checked. The Go version and server binary availability checks are moved to the health module to run when starting the plugin. Co-authored by: jakub.bortlik@proton.me * fix: set diffview hook for added files (#573) The hook for setting up keymaps and autocommands didn't fire correctly if the first file in the review was a new file, so it was diffed against diffview://null, which caused some autocommands to not fire appropriately or some state (reviewer.tabid) not to be set correctly. This PR also removes an unnecessary diagnostics refresh. * docs: improve type annotations and user documentation (#574) Adds function annotations throughout the whole codebase to increase maintainability and ease of contribution. The README.md and doc/gitlab.nvim.txt are updated to correspond to the current recommended installation process. Removes several unused functions and one whole unused module (hover.lua). Also adds several TODOs and FIXMEs for issues discovered during the codebase review. * docs: fix typos (#575) * fix: use pagination to list project members (#579) Use `gitlab.Scan` to paginate through all project members. Co-authored-by: Farsan <farsan.rashid@agoda.com> * fix: hunk header counts and the deleted-line check (#580) Commenting on the unchanged line directly above a deletion of exactly one line sent it as an added line, anchored to the new side, and without the warning that comments on unmodified lines are placed in the old file. It now goes to the old file with both line numbers, the way it already did above a deletion of two or more lines. The check for deleted lines implemented the general algorithm for arbitrary diffs, including context lines, and its range carried an off-by-one that no caller could reach. The only diff it ever sees comes from git.diff_files, which runs with --unified=0 and has no context lines, so a line is removed exactly when it falls in the hunk's old range. Simplify to that rather than fix the bound. * fix: close an orphaned discussion window `NuiSplit:unmount` sets an internal loading flag before destroying buffer and window and clears it only at the end, so an error in between leaves the flag set and every later unmount returns early without closing anything. `close` marked the split as gone regardless, so the window can stay on screen while `split_visible` says otherwise and the next toggle opens a second one beside it. No such failure was observed, that part is hardening. Close the window directly when the split does not, and keep `split_visible` set for as long as the window is alive. The WinClosed handler defers the teardown to the next tick. A buffer wiped from inside a WinClosed callback fires no BufWipeout, and the autocmds that reset `linked_bufnr` and `unlinked_bufnr` hang off that event, so a synchronous teardown leaves both fields holding the number of a wiped buffer. * fix: release the discussion buffers on close The linked and unlinked buffers are created per open, not per session, so the pair the closing window leaves behind stays listed forever while the next open allocates a fresh one: two leaked buffers per open/close cycle. * fix: guard get_root_node against a nil parent A node of type "note" without `is_root` recurses into `tree:get_node(nil)`, which resolves a node from a window's cursor and can hand back the very same node. The call is in tail position, so the recursion never overflows the stack, it freezes Neovim. Give up on a nil parent, the way get_note_node already does. * fix: get correct location data for comments efficiently * docs: replace number with integer in type annotations * feat: execute a callback on all run_job paths Previously a transport failure, an empty response body, or unparseable JSON returned without running any callback, leaving callers unable to tell that the request had finished. Now exactly one callback runs for every outcome, including curl failing to spawn. * refactor: extract anonymous on_exit from run_job The handler was a nested anonymous function, which obscured the outcome branches and could not be exercised directly. Move it to M._make_on_exit and cover the branches with specs. * refactor: rename job.lua to client.lua The module sends requests to the Go server rather than managing jobs, so name it for what it does: run_job becomes send_request, and the spec follows. * fix: redact request body from notification Redact the request body from the notifications that echo the curl command, since it can carry comment and MR text. * fix: reject response bodies that are not objects * refactor: rename details to error in ErrorResponse "error" states what the field holds, which reads better on the Lua side where it gates the on_error callback. This changes the wire format, so a response carrying the old key is still accepted, with a warning to rebuild, for users who supply their own binary via server.binary_provided. * feat: show time since last update in winbar even when updating last_updated doubled as the in-flight marker, so the timestamp disappeared while a refresh ran. Track the two states separately, and drive the spinner phase from vim.uv.hrtime so it advances with wall-clock time rather than with the number of winbar renders. * feat: use counter for updates A single Boolean `updating` flag stopped the spinner as soon as the first of two overlapping refreshes finished, suggesting all work was done. An integer counter updates for each refresh individually, so the spinner runs until the last one completes. --------- Co-authored-by: Clement-Hue <47351376+Clement-Hue@users.noreply.github.com> Co-authored-by: Clement Hue <clement.hue@thetrainline.com> Co-authored-by: Mathias Eek <51080320+Cliffback@users.noreply.github.com> Co-authored-by: Sebastian Flügge <seflue@web.de> Co-authored-by: Sebastian Flügge <952313+seflue@users.noreply.github.com> Co-authored-by: Jakub F. Bortlík <jakub.bortlik@proton.me> Co-authored-by: JonLD <73548328+JonLD@users.noreply.github.com> Co-authored-by: Artem Starikov <contact@snejugal.ru> Co-authored-by: Farsan Rashid <farsan.rashid@gmail.com> Co-authored-by: Farsan <farsan.rashid@agoda.com>
Closes #497.
This is still in draft because I would like to test it thoroughly, butthe main functionality works. The maintained Diffview fork supports updating a diffview, and programmatically modifying selections which enables us to update the reviewer in place without closing and reopening after a rebase.There will be a trivial conflict with #530 because both PR's add keybindings in the same place in settings.I've also replaced some synchronous system calls to
gitwith async calls. This prevents blocking the UI (cursor and winbar updates) when checking if the local branch is up-to-date with the remote tracking branch. This makes loading the summary and rebuilding the discussion tree more responsive.AI use: The Go code is mostly generated by Claude Opus 4.5. The same model has been used for reviewing the lua code.