lambdalisue / vim-gina

👣 Asynchronously control git repositories in Neovim/Vim 8
http://www.vim.org/scripts/script.php?script_id=5531
MIT License
688 stars 27 forks source link

Gina log on Windows #4

Closed lambdalisue closed 7 years ago

lambdalisue commented 7 years ago

https://github.com/lambdalisue/gina.vim/issues/1#issuecomment-275993898 https://www.youtube.com/watch?v=Dg1S83ajz68&feature=youtu.be

OS: Windows Vim: 8.0.160

lambdalisue commented 7 years ago

@bimlas Just I wonder, what about :Gina log --async?

lambdalisue commented 7 years ago

I guess this commit (https://github.com/lambdalisue/gina.vim/commit/1897658209c3acf6545c0f79f52d4f16a94acce8) fix the issue. Could you confirm @bimlas?

bimlas commented 7 years ago

Nice work! It shows the full log immediately. 👍 --async works too.

But a bit problem remains: as you can see in the video, the log is opened in "full screen" (does not creates a split, but reuses the current window) and hitting <CR> on an entry opens the current file's version in that commit instead of the commit itself.

Tried Gina log -- ":/" (to show the log of repository root) without success:

fatal: :\: '\' is outside repository

I thougth this is solved at here.

Tried Gina log -- "%:p:h" too what is a bit weird: it opens a split, but hitting <CR> on an entry makes the main window blank, the status line is

gina://gina.vim:show/199ca20:C:/users/Laci/.vim/plugins/gina.vim

Same behaviour with --async.

lambdalisue commented 7 years ago

But a bit problem remains: as you can see in the video, the log is opened in "full screen" (does not creates a split, but reuses the current window)

Hum... It is a bit strange. Could you show me the result of :echo gina#core#meta#get('args').params on gina:xxxxx:log window?

hitting on an entry opens the current file's version in that commit instead of the commit itself.

I don't get this sentence. Could you explain this a bit more?

I might mis-understood rest of what you said but does https://github.com/lambdalisue/gina.vim/commit/4a2b7b5ec30338089397cecadc35efc1d008a767 solve the issue?

bimlas commented 7 years ago
:echo gina#core#meta#get('args').params
{'group': 'short', 'async': 0, 'opener': '12split', 'cmdarg': ' ', 'path': ''}

I don't get this sentence. Could you explain this a bit more?

Opened Gina log, then pressing <CR> on commits: https://youtu.be/Dg1S83ajz68?t=37

Rest of my commit: I tried to list the log of the repo like Gina log should by adding a path for it, especially the repo's path. In a git repository git log and git log -- ":/" producing the same (the log of the whole repository), because :/ means the root of the repository: https://git-scm.com/docs/gitglossary#def_pathspec

4a2b7b5 drops the same errormessage (fatal: :\: '\' is outside repository).

bimlas commented 7 years ago

I think I misunderstood something.

:echo gina#core#meta#get('args').params 

On Gina log

{'group': 'short', 'async': 0, 'opener': '12split', 'cmdarg': ' ', 'path': ''}

On buffer opened by <CR> in Gina log

{'path': './README.md', 'group': '', 'async': 0, 'opener': 'edit', 'line': v:null, 'cmdarg': ' ', 'object': '4a2b7b5:./README.md', 'repository': 0, 'col': v:null, 'commit': '4a2b7b5'}
bimlas commented 7 years ago

But it's not related to this issue. Should I copy these comments to #1?

lambdalisue commented 7 years ago

Hum.... The expected behaviour is

https://asciinema.org/a/101748

Note that I custom the default behaviour as

call gina#command#custom('log', '--async')
call gina#command#custom('log', '--opener', 'vsplit')

I'm getting a bit confused. The followings are my expect and which point should I focus?

  1. Gina log --opener=split should open gina:xxxxx:log window with split command.
  2. Hitting <CR> on gina:xxxxx:log will execute show action
    1. If the buffer name is something like gina:xxxxx:log/:README.md (Gina log -- %), it shows a content of README.md at a commit under the cursor
    2. If the buffer name is gina:xxxxx:log (Gina log), it shows a commit message and diff at a commit under the cursor
  3. Gina log -- :\ or Gina log -- :/ was not expected. What the difference between Gina log?
lambdalisue commented 7 years ago

But it's not related to this issue. Should I copy these comments to #1?

It's OK. It seems the problem is on Gina log

bimlas commented 7 years ago

Tried out on gina.vim/README.md at 9367496.

  1. Gina log --opener=split works as you can see in the screencast above.

    :echo gina#core#meta#get('args').params
    {'group': 'short', 'async': 0, 'opener': 'split', 'cmdarg': ' ', 'path': ''}

    The buffer name is gina:gina.vim:log\ (as bufname('%') says)

  2. <CR> opens the commit in the same (fullscreen) window (shows the commit message and diffs).

    {'path': '', 'group': '', 'async': 0, 'opener': 'edit', 'line': v:null, 'cmdarg': ' ', 'object': '9367496', 'repository': 0, 'col': v:null, 'commit': '9367496'}

    The buffer name is gina://gina.vim:show/9367496

  3. Gina log -- % works as expected: splits the window, <CR> shows the file at commit under the cursor.

  4. See 2.

  5. To be clear: it's absolutely no sense to use git log -- :/ instead of git log (because it producing the same output), but I thought passing a filename to Gina log solves the issue.

Gina log -- :/ works as above.

BTW sometimes I using for example git log --name-status -- ':/' ':!/**.vim' to see the log of non-Vim files: it shows the log including every file (':/'), excluding *.vim (':!/**.vim). So parsing :/ is nice to have.

bimlas commented 7 years ago

Just created a test - it should pass on Appveyor.

bimlas commented 7 years ago

Here's my output of themis:

Failures:
  1) Acceptance test log opens a new split and shows the log of repository
     Error occurred line:
       2:       Assert Equals(winnr('$'), 2)
     The equivalent values were expected, but it was not the case.

         expected: 2
              got: 1

  2) Acceptance test log opens a new split and shows the log of file
     Error occurred line:
       2:       Assert Equals(winnr('$'), 2)
     The equivalent values were expected, but it was not the case.

         expected: 2
              got: 1
bimlas commented 7 years ago

Updated the tests, now (19406fa) the output is:

Failures:
  1) Acceptance test log opens a new split and shows the log of repository
     Error occurred line:
       2:       Assert Equals(winnr('$'), 2)
     The equivalent values were expected, but it was not the case.

         expected: 2
              got: 1

  2) Acceptance test log opens a new split and shows the log of file
     Error occurred line:
       3:       Assert Match(bufname('%'), '^gina:.*:log/:README.md$')
     Match was expected, but did not match.

         target: 'gina:gina.vim:log\:README.md'
         pattern: '^gina:.*:log/:README.md$'
lambdalisue commented 7 years ago

I'll look this at day after tommorow thanks!

lambdalisue commented 7 years ago

https://ci.appveyor.com/project/lambdalisue/gina-vim/build/75

It seems the bug is fixed. Could you try https://github.com/lambdalisue/gina.vim/commit/4fc2cac28eeb29e6cdf8e62179b5cf032ad0ced6

bimlas commented 7 years ago

The issues above are solved:

The plugin works as expected and seen on the screencast.

Still exists:

The backslash should be forward slash.

The last one is not related to this issue, but I explaining it here.

Well, yeah, it's an expected behaviour because the log shows the history of a DIRECTORY instead of a file, thus it cannot show the content of it, just a blank buffer.

I don't know how do you using Git, but if I want to view a log of a file, (most of the time) I don't interested in the contents of it, but the modified lines. So I think the default action for Gina log -- file should be the show of commit (and jumping to the hunk of the file). This would make Gina log behave like git log.

lambdalisue commented 7 years ago

Gina log -- ":/" (to show the log of repository root) drops fatal: :\: '\' is outside repository

It is a kind new feature. Please make a new issue as a feature request 👍

Gina log -- "%:p:h" opens a split, but hitting <CR> on an entry makes the main window blank

This one as well. But as you mentioned, what is the use-case or expected output of this one?

So I think the default action for Gina log -- file should be the show of commit (and jumping to the hunk of the file). This would make Gina log behave like git log.

Well, I'm not sure if it should be. While Gina log -- {path} is used to see a history of the particular file, I feel showing a repository commit (content showed by git show {commit}) is a bit redundant. What do you think about to make a default action to diff instead?

bimlas commented 7 years ago

Git is a version controlling system which tracks the modifications, not a backup software where the user only wants to view/restore an earlier state of the file. This is why git log has a --patch flag (which shows the diff besides the commit message), but none for "show all versions of file". Plus --patch can be used as -p because it's used quite often.

I think it should show (at least) git show SHA -- FILE (which prints the commit message too) instead of git show SHA:FILE (showing only the file itself). From here the commit (and the corresponding changes) can be opened by pressing <CR> on SHA (same behavior as Gina log without filename).

lambdalisue commented 7 years ago

Make sence. Could you create a new issue for that as well while that is a new request and not a bug

Implemented. https://github.com/lambdalisue/gina.vim/commit/9c7c435d6b28a2a75fb0a00c8081988b270ee39b

bimlas commented 7 years ago

👍 Cool, it made Gina log -- %:p:h sence!