NeogitOrg / neogit

An interactive and powerful Git interface for Neovim, inspired by Magit
MIT License
3.71k stars 221 forks source link

`b o` fails for github.com (${repository} incorrectly replaced) #1248

Closed franklouwers closed 3 months ago

franklouwers commented 3 months ago

Description

I am on nvim 0.10 nightly. I want to use the new b o feature to open my browser at the "Pull Request" page for a branch.

For Github, there is incorrect parsing of the git_services setup section resulting in malformed URLs.

My NeoGit config is as follows, using Lazy as a loader, but the same thing happens with the minimal.lua config below.

  {
    "NeogitOrg/neogit",
    branch = "nightly",
    dependencies = {
      "nvim-lua/plenary.nvim",  -- required
      "sindrets/diffview.nvim", -- optional - Diff integration

      -- Only one of these is needed, not both.
      "nvim-telescope/telescope.nvim", -- optional
      --    "ibhagwan/fzf-lua",              -- optional
    },
    cmd = 'Neogit',
    config = function()
      local neogit = require('neogit')
      neogit.setup({
        use_per_project_settings = false,
        graph_style = 'unicode',
        git_services = {
          ["github.com"] = "https://github.com/${owner}/${repository}/compare/${branch_name}?expand=1",
          ["bitbucket.org"] = "https://bitbucket.org/${owner}/${repository}/pull-requests/new?source=${branch_name}&t=1",
          ["gitlab.com"] =
          "https://gitlab.com/${owner}/${repository}/merge_requests/new?merge_request[source_branch]=${branch_name}",
          ["gitlab.PRIVATE_INSTANCE_MASKED"] =
          "https://gitlab.PRIVATE_INSTANCE_MASKED/${owner}/${repository}/merge_requests/new?merge_request[source_branch]=${branch_name}",
        },
      })
    end,

  }

The url opened is https://github.com/my-org/$%7Brepository%7D/compare/feature/karpenter-max-lifetime?expand=1

As you can see, the ${owner} is correctly parsed, but the ${repository} isn't.

I've tried this with both the Stable and the Nightly branch Neogit.

Neovim version

NVIM v0.10.0-dev-2734+gdde2cc65f-Homebrew Build type: Release LuaJIT 2.1.1710088188

Operating system and version

macOS 14.4.1

Steps to reproduce

  1. nvim -nu minimal.lua in a directory with contains a git clone of a github.com repo where you can create PRs and push to branches
  2. switch to a branch, change a file, commit the change
  3. P u in Neogit to push the branch
  4. b o in Neogit to open the PR page

Expected behavior

Open the PR webpage on github.com

Actual behavior

Get a Github 404 because ${repository} isn't correctly parsed

Minimal config

-- NOTE: See the end of this file if you are reporting an issue, etc. Ignore all the "scary" functions up top, those are
-- used for setup and other operations.
local M = {}

local base_root_path = vim.fn.fnamemodify(debug.getinfo(1, "S").source:sub(2), ":p:h") .. "/.min"
function M.root(path)
  return base_root_path .. "/" .. (path or "")
end

function M.load_plugin(plugin_name, plugin_url)
  local package_root = M.root("plugins/")
  local install_destination = package_root .. plugin_name
  vim.opt.runtimepath:append(install_destination)

  if not vim.loop.fs_stat(package_root) then
    vim.fn.mkdir(package_root, "p")
  end

  if not vim.loop.fs_stat(install_destination) then
    print(string.format("> Downloading plugin '%s' to '%s'", plugin_name, install_destination))
    vim.fn.system({
      "git",
      "clone",
      "--depth=1",
      plugin_url,
      install_destination,
    })
    if vim.v.shell_error > 0 then
      error(string.format("> Failed to clone plugin: '%s' in '%s'!", plugin_name, install_destination),
        vim.log.levels.ERROR)
    end
  end
end

---@alias PluginName string The plugin name, will be used as part of the git clone destination
---@alias PluginUrl string The git url at which a plugin is located, can be a path. See https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols for details
---@alias MinPlugins table<PluginName, PluginUrl>

---Do the initial setup. Downloads plugins, ensures the minimal init does not pollute the filesystem by keeping
---everything self contained to the CWD of the minimal init file. Run prior to running tests, reproducing issues, etc.
---@param plugins? table<PluginName, PluginUrl>
function M.setup(plugins)
  vim.opt.packpath = {}                      -- Empty the package path so we use only the plugins specified
  vim.opt.runtimepath:append(M.root(".min")) -- Ensure the runtime detects the root min dir

  -- Install required plugins
  if plugins ~= nil then
    for plugin_name, plugin_url in pairs(plugins) do
      M.load_plugin(plugin_name, plugin_url)
    end
  end

  vim.env.XDG_CONFIG_HOME = M.root("xdg/config")
  vim.env.XDG_DATA_HOME = M.root("xdg/data")
  vim.env.XDG_STATE_HOME = M.root("xdg/state")
  vim.env.XDG_CACHE_HOME = M.root("xdg/cache")

  -- NOTE: Cleanup the xdg cache on exit so new runs of the minimal init doesn't share any previous state, e.g. shada
  vim.api.nvim_create_autocmd("VimLeave", {
    callback = function()
      vim.fn.system({
        "rm",
        "-r",
        "-f",
        M.root("xdg")
      })
    end
  })
end

-- NOTE: If you have additional plugins you need to install to reproduce your issue, include them in the plugins
-- table within the setup call below.
M.setup({
  plenary = "https://github.com/nvim-lua/plenary.nvim.git",
  telescope = "https://github.com/nvim-telescope/telescope.nvim",
  diffview = "https://github.com/sindrets/diffview.nvim",
  neogit = "https://github.com/NeogitOrg/neogit"
})
-- WARN: Do all plugin setup, test runs, reproductions, etc. AFTER calling setup with a list of plugins!
-- Basically, do all that stuff AFTER this line.
require("neogit").setup({}) -- For instance, setup Neogit
CKolkey commented 3 months ago

I use this feature all the time, so I don't think its broken. More likely an edge case with the remote? In your ./.git/config file, whats the url of the remote?

franklouwers commented 3 months ago
[remote "origin"]
    url = https://github.com/my-org/myrepo

replaced the org with my-org and the repository with myrepo to protect clients. All lowercase, no funky chars, the real my-org has a dash in the name (just like my-org) does

franklouwers commented 3 months ago

For completeness, this is my full (unmodified) ~/.gitconfig:

[user]
    email = frank@louwers.be
    name = Frank Louwers
    signingkey = ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPzAulvKp8l2cHRIVhi1c3Z8uprO7UrwENZShOT5S/gu
[gpg]
    format = ssh
[gpg "ssh"]
    program = /Applications/1Password.app/Contents/MacOS/op-ssh-sign
[commit]
    gpgsign = true

# force ssh for github.com and gitlab.com
[url "ssh://git@github.com/"]
    insteadOf = https://github.com/
[url "ssh://git@gitlab.com/"]
    insteadOf = https://gitlab.com/
[init]
    defaultBranch = main
[push]
    autoSetupRemote = true
  default = current

[github]
  user = franklouwers

Would the insteadOf mangle things?

CKolkey commented 3 months ago

Yeah, thats totally possible - I'll try to reproduce the issue and add support for ssh uri's

franklouwers commented 3 months ago

FYI I did some more tests:

1) comment the "insteadOf" section of my gitconfig (see below for full gitconfig) 2) fork this repo to one of my orgs (https://github.com/kiwazo-be/neogit) 3) git clone https://github.com/kiwazo-be/neogit 4) git switch -c feature/test 5) copy and paste minimal.lua from the above (unmodified from the ticket template) as t/minimal.lua 6) start nvim -nu t/minimal.lua 7) start Neogit, add minimal.lua, commit, push (using P u) 8) b o, expect safari to open https://github.com/kiwazo-be/neogit/compare/feature/test?expand=1 9) see safari open https://github.com/kiwazo-be/$%7Brepository%7D/compare/feature/test?expand=1

Am puzzled here...

~/.gitconfig:

[user]
  email = frank@louwers.be
  name = Frank Louwers
  signingkey = ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPzAulvKp8l2cHRIVhi1c3Z8uprO7UrwENZShOT5S/gu
[gpg]
  format = ssh
[gpg "ssh"]
  program = /Applications/1Password.app/Contents/MacOS/op-ssh-sign
[commit]
  gpgsign = true

# force ssh for github.com and gitlab.com
; [url "ssh://git@github.com/"]
;   insteadOf = https://github.com/
; [url "ssh://git@gitlab.com/"]
;   insteadOf = https://gitlab.com/
[init]
  defaultBranch = main
[push]
  autoSetupRemote = true
  default = current

[github]
  user = franklouwers
CKolkey commented 3 months ago

Ah! Ok, it's because repository is nil from this function: https://github.com/neogitorg/neogit/blob/fd63c19287cb275ea38d2c85f41ce712a816c160/lua/neogit/lib/git/remote.lua#L103

Your url doesn't end in .git - suppose I should support when it doesn't

CKolkey commented 3 months ago

5d24c8c (#978) - lemme know if this resolves things for you :)

franklouwers commented 3 months ago

YES, this solves this! Thank you for taking the time to look into this.

CKolkey commented 3 months ago

Very welcome, thanks for letting me know :)

franklouwers commented 3 months ago

I believe this can be closed, as the Nightly update fixes this.