sindrets / diffview.nvim

Single tabpage interface for easily cycling through diffs for all modified files for any git rev.
Other
4.02k stars 111 forks source link

Feature: Support quoted arguments #272

Closed TheodorRene closed 1 year ago

TheodorRene commented 1 year ago

Man page from git-rev-parse:

       [<refname>]@{<date>}, e.g. master@{yesterday}, HEAD@{5 minutes ago}
           A ref followed by the suffix @ with a date specification enclosed in a brace pair (e.g.
           {yesterday}, {1 month 2 weeks 3 days 1 hour 1 second ago} or {1979-02-26 18:30:00})
           specifies the value of the ref at a prior point in time. This suffix may only be used
           immediately following a ref name and the ref must have an existing log
           ($GIT_DIR/logs/<ref>). Note that this looks up the state of your local ref at a given time;
           e.g., what was in your local master branch last week. If you want to look at commits made
           during certain times, see --since and --until.

:DiffviewOpen develop...HEAD@{1 days ago} will not work. From what I gather its because the parsing of symmetric diffs is only based on the first argument of a whitespace split diff. GitAdapter:rev_parse takes develop...HEAD@{1 since GitAdapter:diffview_options gives it only args[0]. The best would be some custom logic to parse the arguments instead of solely on whitespace. I might have a crack at this over the weekend, I'm just a bit unsure where this should be placed.

It could either be mashed in here, not passing state.fargs, but a list after some custom split logic:

-- diffview.lua
command("DiffviewOpen", function(state)
    print(vim.inspect(state))
  diffview.open(unpack(state.fargs))
end, { nargs = "*", complete = completion })

or here:

-- git/init.lua
function GitAdapter:diffview_options(args)
  local default_args = config.get_config().default_args.DiffviewOpen
  local argo = arg_parser.parse(vim.tbl_flatten({ default_args, args }))
  local rev_arg = argo.args[1]

  local left, right = self:parse_revs(rev_arg, {
    cached = argo:get_flag({ "cached", "staged" }),
    imply_local = argo:get_flag("imply-local"),
  })
sindrets commented 1 year ago

Just like in the shell, arguments in the vim command line are whitespace separated. Thus - just like in the shell - if your arguments contain whitespace, it needs to be escaped with a backslash (\):

:DiffviewOpen develop...HEAD@{1\ days\ ago}

You would have to do the same thing in a shell unless you quote your args (something that is not supported natively by the vim command line):

$ git log --oneline develop...HEAD@{1\ days\ ago}
TheodorRene commented 1 year ago

That makes sense. Do you prefer the current behaviour? We are not limited by Vims arguments parsing in theory, but it might be more consistent to keep the same behaviour as in the shell.

sindrets commented 1 year ago

How do you suggest we change it?

TheodorRene commented 1 year ago

Make :DiffviewOpen develop...HEAD@{1 days ago} or :DiffviewOpen develop...HEAD@"{1 days ago}"

equivalent to: :DiffviewOpen develop...HEAD@{1\ days\ ago}

sindrets commented 1 year ago

The first form you suggest is a really bad idea. It's unclear how you intend for arguments to be separated in this form, if not by non-escaped whitespace. And regardless, it would break with conventions of every command line I have ever seen.

However, I'm in favor of supporting quoted args. In fact I have a local branch where I am already experimenting with this.

TheodorRene commented 1 year ago

Sounds good to me 👍