search

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

dev #16

Closed lambdalisue closed 7 years ago

lambdalisue commented 7 years ago
lambdalisue commented 7 years ago

@bimlas I'll merge this branch when all test passed. Could you try if this branch works as expect in Windows? (See the PR note. I summarised what has changed. A lot of feature has changed)

bimlas commented 7 years ago

OMG! :open_mouth: You just rewrote the half of the plugin... Before anything else, let me know you that your work and endurance impressing me. :clap:

Sadly there are issues.

Info about environment

Vim 8.0.0206 Python 2.7.13 Python 3.5.3

.vimrc

set nocompatible
filetype plugin indent on
syntax enable
set runtimepath+=$HOME/.vim/plugins/gina.vim
" Nearly nothing works without this (though my PC is quite fast).
let g:gina#core#writer#updatetime=200

The repo where I doing the tests is https://github.com/bimlas/git-test (README.adoc is opened in Vim) and the vimrc is:

Results

File encoding and format may not treated well to improve the performance

  • Give me a repository with CRLF (fileformat) for test if current implementation has issues

:+1: Gina log works as expected even on bimlas/git-test@47e02fd: there's no ^M on the end of the lines.

  • Give me a repository with non utf-8 encoding for test if current implementation has issues

:-1: My Windows' default encoding is CP1250. Opening a commit with the same encoding works fine, but when it differs (showing the commit of UTF8 file), it drops this:

Gina log -> open bimlas/git-test@56642cd

Error detected while processing function <SNR>76__timer_callback[6]..115[8]..<SNR>76_extend_content[7]..<SNR>76__extend_content_python3:
line   17:
Traceback (most recent call last):
  File "<string>", line 15, in <module>
  File "<string>", line 2, in _vital_vim_bufferwriter_extend_content
  File "C:\app\python3\Lib\encodings\cp1250.py", line 15, in decode
    return codecs.charmap_decode(input,errors,decoding_table)
UnicodeDecodeError: 'charmap' codec can't decode byte 0x81 in position 2: character maps to <undefined>

Note that encoding is detected: C:\app\python3\Lib\encodings\cp1250.py

Adding set encoding=utf8 to vimrc reversing the problem: UTF-8 works, CP1250 drops error.

Gina log -> open bimlas/git-test@b97ab28

Error detected while processing function <SNR>71__timer_callback[6]..103[8]..<SNR>71_extend_content[7]..<SNR>71__extend_content_python3:
line   17:
Traceback (most recent call last):
  File "<string>", line 15, in <module>
  File "<string>", line 2, in _vital_vim_bufferwriter_extend_content
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe1 in position 1: invalid continuation byte

Sync feature is no longer supported

There's no output in most of the cases without g:gina#core#writer#updatetime.

  • :Gina! status would echo the result a bit later in Vim 8

:confused: Gina! works, but does not clearing itself: called it twice, both result was echoed for the second time. (note for myself: really have to learn English)

"The behaviour of gina-commit has drastically changed" & "When python3 or python is available..."

:+1: Related to the previous issue: the output contains only what it has to: the output of the last commit command for example (commited twice after each other). Works as expected.

A minimum behaviour tests for all commands has added

:-1: A lot of test fails, but it's late night here, so I'm going to sleep - I think I will post the results tomorrow.

lambdalisue commented 7 years ago

Wow. Thanks for you investigations! You are the best contributor who I've ever met :+1: So currently, it seems there are

  1. Encoding issue which could be test with bimlas/git-test
  2. Need to increase g:gina#core#writer#updatetime somehow
  3. Message is shown twice for Gina! status (Got why. Will fix)

right? By the way, the test fail may because of missing git config --global core.XXXXX something. I'll fix that soon!

lambdalisue commented 7 years ago

ToDo before merge

aiya000 commented 7 years ago

Maybe this is not related, I cought an exception when :Gina executed without any argument 😄

lambdalisue commented 7 years ago

@aiya000 Thanks, it fixed now by -nargs=+ :+1:

lambdalisue commented 7 years ago

@bimlas I fixed the following three issues. Could you confirm?

  1. :Gina! status no longer shows duplicated messages in Vim (note that you may need to move a cursor to see the message while it use CursorMoved autocmd or similar due to the technical limitation of Vim)
  2. All content is obtained even the process has terminated before data has processed in Vim (I guess this fix g:gina#core#writer#updatetime issue)
  3. A content of gina-status window is no longer wiped out by hitting << or whatever (Did it happen in your environment? In my environment, most of time the content is wiped out by << in Vim but Neovim)

Now, I'm going to fix the encoding issues you point :+1:

bimlas commented 7 years ago

Tested on and f98fe6f:

  1. :+1: Works as expected.
  2. :confused: : I don't see any difference. Just to be sure: the modifications would make everything work without setting up g:gina#core#writer#updatetime at all?
  3. :+1: I didn't happened for me. But << in Gina status didn't updated the status window (had to execute it again) in c839de8, but f98fe6f solved the problem.

Here's the output of themis: https://gist.github.com/bimlas/bbc2f5c6c0232f6f38f3b090f64ded92

I think the fails would be related to g:gina#core#writer#updatetime, but I'm very curious about why these passing on AppVeyor..?

I think you should continue with:

If still failing, then we have to go to the deep.

lambdalisue commented 7 years ago

@bimlas Thanks for the investigations. I'll check these later. Hey, there is a good news. I think the encoding issue has solved. It seems AppVeyor argued with related code but it seems the test has failed by minor difference. So could you try? Note that now :Gina show ++enc=cp1250 or :e ++enc=cp1250 open gina's content with cp1250!!!

          The equivalent values were expected, but it was not the case.

              expected: ['Unix EOL', 'árvĂ­ztűrĹ‘ tĂĽkörfĂşrĂłgĂ©p', 'Ă??RVĂŤZTĹ°RĹ?? TĂśKĂ–RFĂšRĂ“GÉP']
                   got: ['Unix EOL', 'árvíztűrő tükörfúrógép', 'ÁRVÍZTŰRŐ TÜKÖRFÚRÓGÉP']

May Windows's command prompt cannot handle some characters correctly?

lambdalisue commented 7 years ago

I don't see any difference.

I actually don't understand what kind of problem exists when g:gina#core#writer#updatetime = 10.... Well I added an event viewer so could you try? The procedure is

  1. :Gina _events
  2. :Gina status

Then you will find something like

selection_061

The order of the events should be similar. If writer:flushed(...) are shown with some value but nothing comes up to the buffer, we need deep investigation for this problem.

lambdalisue commented 7 years ago

I've found some points and tried to fix on 94671ff

pre  : 09:40:34.493079: process:registered(17409)       <gina://gina.vim:status>
post : 09:40:34.494057: process:registered(17409)       <gina://gina.vim:status>
pre  : 09:40:34.497561: process:unregistered(17409)     <gina://gina.vim:status>
post : 09:40:34.498263: process:unregistered(17409)     <gina://gina.vim:status>
pre  : 09:40:34.502413: process:registered(17410)       <gina://gina.vim:status>
post : 09:40:34.503355: process:registered(17410)       <gina://gina.vim:status>
pre  : 09:40:34.504090: writer:started(3)               <gina://gina.vim:status>
post : 09:40:34.504794: writer:started(3)               <gina://gina.vim:status>
pre  : 09:40:34.531260: writer:stopped(3)               <gina://gina.vim:status>
post : 09:40:34.537212: writer:stopped(3)               <gina://gina.vim:status>
pre  : 09:40:34.543725: process:unregistered(17410)     <gina://gina.vim:status>
post : 09:40:34.549777: process:unregistered(17410)     <gina://gina.vim:status>
pre  : 09:40:34.555709: command:called('status')        <gina://gina.vim:status>
post : 09:40:34.561599: command:called('status')        <gina://gina.vim:status>
lambdalisue commented 7 years ago

@bimlas Finally. I think I could figure out the g:gina#core#writer#updatetime issue. It should be fixed on https://github.com/lambdalisue/gina.vim/pull/16/commits/5ad6a7122ef7fe2fb982b21a263d898ef791eb7d

bimlas commented 7 years ago

Tested on 2156206

Fileencoding

:confused: Gina info ++enc=... works very well (the fileencodig of the commits reflects the ++enc parameter), but hitting <CR> on Gina log entry does not sets up the fileencoding of the buffer (which is empty).

.vimrc:

set nocompatible
filetype plugin indent on
syntax enable
set runtimepath+=$HOME/.vim/plugins/gina.vim
set runtimepath+=$HOME/.vim/plugins/vim-scriptease
let g:gina#core#writer#updatetime=200
" set encoding=utf8
set fileformats=unix,dos fileencodings=utf8,cp1250

<CR> on Gina log entry Gina info ++enc=...

g:gina#core#writer#updatetime

:+1: Works as expected, everything is so smooth without setting up the variable! :clap:

Running tests

Additional tests fails, but I just noticed an UFO: Vim without .vimrc (or even with the minimal settings above) does not sets up &termencoding, but on the output of Themis:

&termencoding:  cp852

Well, if Wikipedia is right, then both of them is used in Hungarian Windows (what I using): CP852, CP1250. I thougth that CP1250 is my default encoding because :ser fenc=default | set fenc? results CP1250, but I'm not sure now.

The output on AppVeyor contains &fileencodings: ucs-bom,utf-8,default,latin1 thus I think it have to be a .vimrc already in use with special settings. It's bad to be used in a testing environment. I think the settings come from Vim-Kaoriya, why don't using native Vim? The zipped can be used as is (I think, never used on AppVeyor):

https://github.com/vim/vim-win32-installer/releases

lambdalisue commented 7 years ago

Auto detection for encoding or format requires additional steps. So I'm not going to support that at least with this pr.

24

Also for CI. I'm planning to improve CI env but not with this pr.

23

So anything else? If no, I'll merge this:+1:

bimlas commented 7 years ago

Well, a lot of tests fails and most of the features seems to work. I think you can merge if the failings are related to #23.

lambdalisue commented 7 years ago

I'll work on test later. Finally this new implementation has merged :tada:

lambdalisue commented 7 years ago

@bimlas It seems all test pass on an offical vim from http://www.vim.org/download.php#pc (vim80w32.zip)

https://ci.appveyor.com/project/lambdalisue/gina-vim/build/218/job/6ugsaooce2y94x1s

So I think your environment is a bit special.

bimlas commented 7 years ago

It's outdated (8.0.0069) and does not have any interfaces so the tests are running without async.

bimlas commented 7 years ago

Just tried out Themis with this version of Vim and it fails a lot, but it has these lines on the output, AppVeyor has not:

  2) Vim.BufferWriter .assign_content({bufnr}, {content}) python returns 0 if a buffer with {bufnr} does not exist
     This Vim does not support python interface

AppVeyor should drop the same, I think.

EDIT Forget about this, it was my fault.

bimlas commented 7 years ago

You're right, don't worry about the tests, I will explain soon.

lambdalisue commented 7 years ago
  1. I could not find any later official release for Windows. I don't want to compile on AppVeyor... That's why the old version is used
  2. gina.vim should work without python while it use job feature. python interface is required for seamless buffer writing and that is not a part of the job feature.
lambdalisue commented 7 years ago

Detail about "seamless buffer writing"

While Vim does not provides setbufline like function (but getbufline :confused:), the target buffer must be focused before updating the content. This makes a things a bit slow so I used python interface. The python interface provides buffers list which can directly access the content of an arbitrary buffer and that does not require changing focus temporary.

Even without python interface, gina.vim focus the target window and restore the original focus really quick so users usually does not notice if the focus has just changed and restored.