Feat add squash and removesourcebranch to createmrrequest

[jakubbortlik]

This MR addresses issue #222 by:

1. Showing the delete_branch and squash information in the MR Summary.
2. Enabling setting these options in the Create MR popup
3. Enabling setting default values for these options in settings.create_mr
4. Adding the necessary options in cmd/create_mr.go

It also makes some minor additional changes:

1. Cleanup of docs (especially the gitlab.create_mr parameters)..
2. Some refactoring of Box popup settings to make adding new options easier.
3. Some refactoring of the pick_template and pick_target functions to make adding new options easier.

I think some other changes should be implemented in order to improve the user experience but they are not part of this MR:

1. The behaviour of gitlab.merge() should be adjusted. Now the equally named options of gitlab.merge() simply override the options set by gitlab.create_mr() or selected in the browser (possibly by another user). I believe, by default, merge() should respect the settings that can be seen in the "Summary" (either set by gitlab.create_mr() or in the browser). I can imagine a situation where I wanted to override these, so maybe gitlab.merge() could still be passed arguments when called from the command line or through a mapping, but I would suggest to remove the default settings.merge.squash and settings.merge.delete_branch options, however, this would be a breaking change.
2. The confirmation popup now consists of 5 individual popups. Navigation with <ctrl-w><ctrl-w> and <ctrl-w>W is now more cumbersome than before, so I think it would be nice to implement some cycling mappings requested in the rejected issue Mappings to change focus #219. Then it would be easy to also use these mappings for the Summary popup.
3. Create linewise_actions that would enable changing the target branch and delete_branch and squash options in the confirmation popup, and also some select values in the Details window in the MR Summary. For the confirmation popup this is not that crucial -- a user can start over again if they make a mistake, but as soon as a MR is created, changes can only be made to it in the Browser, so making it possible to adjust some values in the Summary popup would be very practical.
4. Remove the current branch from the possible merge targets in create_mr.pick_target() - this could be done globally by adding a parameter to the following function, something like: utils.get_all_git_branches(remote, include_current_branch) function. Otherwise an error is thrown only after all the MR creation work has been done.
5. Update the MR Summary after values in the Details change (e.g., reviewers, assignees, status, etc.)

@harrisoncramer, if you think any of these additions make sense I can open feature requests for them, or separate MRs directly.

[harrisoncramer]

Thank you for writing this MR! It looks very nice, I'll take a look this weekend

[harrisoncramer]

This looks great, left a couple of small suggestions.

The other thing that we have to do as part of this MR is remove the squash_message option which is currently present. I think it's just a matter of addressing these three references, since this is now baked into the MR itself:

[harrisoncramer]

These types of statements can be converted into ternary style values in Lua which is a bit easier to read, in my opinion:

local delete_branch = mr.delete_branch ~= nil and mr.delete_branch or state.settings.create_mr.delete_branch

I'd change a variety of if/else statements in this MR to this style, it makes clear that this is just an assignment, eliminates variable shadowing, etc.

[jakubbortlik]

I actually tried the way you suggest before, but the problem is that it does not work for Boolean values. If mr.delete_branch is false it's not able to override the default value state.settings.create_mr.delete_branch if that is true.

How about extracting the logic to a utils function like this:

---Get the first non-nil value in the input table or nil
---@param values table The list of input values
---@return any
M.get_first_non_nil_value = function(values)
  for _, val in pairs(values) do
    if val ~= nil then
      return val
    end
  end
end

Then I'll be able to simplify my code to:

  local delete_branch = u.get_first_non_nil_value({mr.delete_branch, state.settings.create_mr.delete_branch})

Which is quite explicit about what it does.

[jakubbortlik]

By the way, I wanted to add tests for this function in tests/spec/util_spec.lua, but when running busted tests/spec/util_spec.lua I get the following error, followed by all the errors resulting from gitlab.utils not being loaded properly:

Failure → tests/spec/util_spec.lua @ 2                                                                                 
utils/init.lua Loads package                                                                                           
tests/spec/util_spec.lua:4: Expected objects to be the same.                                                           
Passed in:                                                                                                             
(boolean) false                                                                                                        
Expected:                                                                                                              
(boolean) true

Could you please add information in .github/CONTRIBUTING.md about how to run the tests?

[jakubbortlik]

This looks great, left a couple of small suggestions.

The other thing that we have to do as part of this MR is remove the squash_message option which is currently present. I think it's just a matter of addressing these three references, since this is now baked into the MR itself:

I don't think this is correct. The state.settings.popup.squash_message setting is for customizing the squash message popup and I think we need to keep that. state.settings.merge.squash on the other hand, defines whether commits should be squashed, but I think we should get rid of the whole state.settings.merge option, like this:

diff --git a/README.md b/README.md
index 5801d69..b41ac71 100644
--- a/README.md
+++ b/README.md
@@ -201,10 +201,6 @@ require("gitlab").setup({
     success = "✓",
     failed = "<U+F467>",
   },
-  merge = { -- The default behaviors when merging an MR, see "Merging an MR"
-    squash = false,
-    delete_branch = false,
-  },
   create_mr = {
     target = nil, -- Default branch to target when creating an MR
     template_file = nil, -- Default MR template in .gitlab/merge_request_templates
diff --git a/doc/gitlab.nvim.txt b/doc/gitlab.nvim.txt
index 2d7442f..b37b34f 100644
--- a/doc/gitlab.nvim.txt
+++ b/doc/gitlab.nvim.txt
@@ -230,10 +230,6 @@ you call this function with no values the defaults will be used:
         success = "✓",
         failed = "<U+F467>",
       },
-      merge = { -- The default behaviors when merging an MR, see "Merging an MR"
-        squash = false,
-        delete_branch = false,
-      },
       create_mr = {
         target = nil, -- Default branch to target when creating an MR
         template_file = nil, -- Default MR template in .gitlab/merge_request_templates
diff --git a/lua/gitlab/actions/merge.lua b/lua/gitlab/actions/merge.lua
index 324dc1e..b610c8e 100644
--- a/lua/gitlab/actions/merge.lua
+++ b/lua/gitlab/actions/merge.lua
@@ -17,7 +17,7 @@ end

 ---@param opts MergeOpts
 M.merge = function(opts)
-  local merge_body = { squash = state.settings.merge.squash, delete_branch = state.settings.merge.delete_branch }
+  local merge_body = { squash = state.INFO.squash, delete_branch = state.INFO.delete_branch }
   if opts then
     merge_body.squash = opts.squash ~= nil and opts.squash
     merge_body.delete_branch = opts.delete_branch ~= nil and opts.delete_branch
diff --git a/lua/gitlab/state.lua b/lua/gitlab/state.lua
index 61e0684..594dd72 100644
--- a/lua/gitlab/state.lua
+++ b/lua/gitlab/state.lua
@@ -87,10 +87,6 @@ M.settings = {
       return " " .. discussions_content .. " %#Comment#| " .. notes_content .. help
     end,
   },
-  merge = {
-    squash = false,
-    delete_branch = false,
-  },
   create_mr = {
     target = nil,
     template_file = nil,

Then, I think, during a merge the MR settings on Gitlab will be respected by default, but it will be possible to override them in the function call require("gitlab").merge({ squash = false, delete_branch = false }). Of course, I'd also have to adjust documentation accordingly.

[harrisoncramer]

@jakubbortlik All of your proposals look good to me, if you push up I'll approve. I'll take a look at adding some documentation for the Lua test suite soon

[jakubbortlik]

Hi Harrison, I've applied the three changes as agreed (got rid of state.settings.merge, moved metadata in Confirmation popup, and used a new utils function for getting non-nil values), but I've also taken the liberty to modify the doc/gitlab.nvim.txt file (commit 8e1ac27e) to make the formatting more consistent, to make it easier to format the description of API function parameters with the expandtab and sw=4 options. And I've also used "column heading" highlighting with tilde for individual API functions, as previously, I found orientation in the API reference rather difficult. I hope you don't mind these changes.