vim-flog icon indicating copy to clipboard operation
vim-flog copied to clipboard

[vim][windows] Issues with carriage returns (^M)

Open DanSM-5 opened this issue 1 year ago • 12 comments

Description

On vim (no neovim) in windows there seem to be a couple of issues related to carriage returns.

  1. All lines in the graph :Flog end with a ^M character

image

This is small visual effect.

  1. Pressing enter <CR> on any commit fails to open it in fugitive.

image

It looks like some carriage return is getting appended to the hash. This result on a vertical split with an empty buffer.

I tested neovim as well and everything seems to work fine there.

DanSM-5 avatar Sep 13 '24 22:09 DanSM-5

Thanks very much for reporting.

Are you using g:flog_use_internal_lua and using Lua that's compiled with Vim?

What do you get from :lua print(_VERSION) and :lua print(jit.version)?

Appears that Lua's io.popen is system-independent, which is probably what's causing this issue:

This function is system dependent and is not available on all platforms.


I don't have a raw Windows environment to compare against working Linux environments. If anyone can compare tell me how to detect a Windows environment in Lua that's going to include carriage returns, I'm happy to implement a fix. It appears Neovim works, so it might not be all Windows environments, please be careful not to include false matches. It might come down to LuaJIT vs Lua, but I don't know what versions are installed yet. PRs also accepted.

rbong avatar Sep 14 '24 00:09 rbong

I'm using vim from scoop which is compiled with lua 5.4 (vim --version -DDYNAMIC_LUA_DLL=\"lua54.dll\")

:lua print(_VERSION)
Lua 5.4

I think I may have misinterpreted this part of the readme

In Vim, LuaJIT 2.1 must be installed. Lua 5.1 is also supported but less performant than LuaJIT.

as in thinking that the minimum version was lua 5.1 but searching about it makes it look like there are quite some differences between lua versions.


Are you using g:flog_use_internal_lua and using Lua that's compiled with Vim?

I didn't know about this variable, so no, I didn't use it. This version of vim needs the lua54.dll in the path (or same directory) or manually setting it with let &luadll = path/to/lua54.dll which I did some time ago.

Appears that Lua's io.popen is system-independent, which is probably what's causing this issue:

This function is system dependent and is not available on all platforms.

It seems that the function exists :lua print(io.popen) -- function: 00007ffbedb25df0. Is the output directly read from the file handle? Curious if that returns the carriage returns.


With all that, does it means that the plugin only supports lua 5.1?

DanSM-5 avatar Sep 14 '24 01:09 DanSM-5

Yes, the plugin only supports Lua 5.1 unfortunately, because this is the version used by LuaJIT and Neovim. I recommend using LuaJIT if you can, as it is much faster. It does appear to be supported on Windows. It is the only supported version because of the previously mentioned Neovim support and speed.

As you've noted, there are significant differences between Lua versions. This is why LuaJIT is still using Lua 5.1 - it is heavily optimized and it's very difficult to add so many changes.

Please let me know if using Lua 5.1 or LuaJIT resolves your issue.

rbong avatar Sep 14 '24 03:09 rbong

I've experimented a bit and I have some interesting findings.

Using lua 5.4

Using my vim version that has lua 5.4 and setting g:flog_use_internal_lua = 1 works. The issue I was seeing was due to calling lua as an external process which sometimes have that effect of appending the carriage return character. At least is very common for me when calling system().

Using luajit

I tried installing luajit as suggested. It didn't work. Calling :Flog produces an error like this

C:\Users\daniel\scoop\apps\luajit\current\bin\luajit.exe: ...daniel\\vim-config\\plugged\\vim-flog/lua/flog/graph.lua:62
2: table overflow^M^@stack traceback:^M^@^I...daniel\\vim-config\\plugged\\vim-flog/lua/flog/graph.lua:622: in function
'flog_get_graph'^M^@^I...aniel\vim-config\plugged\vim-flog/lua/flog/graph_bin.lua:5: in main chunk^M^@^I[C]: at 0x7ff794
721fe0^M^@1^M
Error detected while processing function flog#cmd#Flog[19]..flog#floggraph#buf#Update[21]..flog#graph#vim#Get[14]..funct
ion flog#cmd#Flog[19]..flog#floggraph#buf#Update[21]..flog#graph#vim#Get[10]..FlogGetVimBinGraph[37]..flog#shell#Run:
line    4:
E605: Exception not caught: flog: encountered shell error

Curious that luajit encounter that error while lua 5.4 (as an external process) didn't.

Maybe it would be worth to document that on windows the ^M symbol can appear when calling lua externally.

For me using the embedded lua that solves the issue (although not the supported version). Thanks for the patience, I got a bit confused with all the lua differences.

Let me know if you would like help to investigate why luajit doesn't work. If not, feel free to close the issue.

DanSM-5 avatar Sep 14 '24 18:09 DanSM-5

Maybe it would be worth to document that on windows the ^M symbol can appear when calling lua externally.

Unfortunately it's likely not that simple and things may still be breaking invisibly if this happens. I can't think of a single feature that won't be broken in some way by this. So PRs and help still welcome for this bug. I'm going to leave it open, you're welcome to unsubscribe.

I tried installing luajit as suggested. It didn't work. Calling :Flog produces an error like this

Might have to do with recent changes to reduce reliance on Fugitive, which has better (and more complicated) shell escaping functions (that especially apply on Windows) which I'm still unravelling and learning from. If you can switch to the v3.0.0 tag and test again if it works I would appreciate it, but if not I understand that this issue is mostly solved for you.

rbong avatar Sep 14 '24 22:09 rbong

Tag 3.0.0 with luajit shows flog buffer like this:

image

DanSM-5 avatar Sep 15 '24 19:09 DanSM-5

Tag 3.0.0 with luajit shows flog buffer like this

Thanks. FYI I believe the path issue that caused the LuaJIT error may be fixed. But I haven't tested it on Windows, and it's not very useful until I can fix the carriage return issue.

rbong avatar Sep 18 '24 00:09 rbong

I pushed fixes that should reliably remove carriage returns on Windows. Let me know if there are still issues. Thanks again for reporting this bug.

rbong avatar Sep 18 '24 01:09 rbong

I've tested with d58ee18 and the results are the same as posted before with the tag 3.0.0.

That means that the path issue is indeed fixed but there is no graph nor commit information. It only displays the dates.

DanSM-5 avatar Sep 20 '24 00:09 DanSM-5

Sorry, I missed that. The v3.0.0 screenshot looked fine to me at a glance before. I will take a look.

PRs welcome for this issue though because I will have to set up a Windows testing environment.

rbong avatar Sep 20 '24 01:09 rbong

Since it's the same as v3.0.0, does that mean it still shows a carriage return at the end of every line?

rbong avatar Sep 20 '24 01:09 rbong

Yes, same carriage return at the end

image image

DanSM-5 avatar Sep 20 '24 01:09 DanSM-5

I tried investigating further on this.

The issue with the carriage returns in the output seems related with the call to systemlist() for which the help already warns about.

:h systemlis
...
Note that on MS-Windows you may get trailing CR characters

However that is unrelated to the bad output showed in the screenshot of my previous comment.

I captured the output of the flog command which is calling lua/flog/graph_bin.lua

flog#shell#Systemlist_output.log

If you open the file, you'll see that the output is the one it shows in my screenshot previously.

I tried running the generated command manually from the command line but I get an error.

luajit "C:\Users\daniel\vim-config\plugged\vim-flog/lua/flog/graph_bin.lua" 0 "__START" false false true false "git -C C:/Users/daniel/.usr_conf --git-dir C:/Users/daniel/.usr_conf/.git log --parents --topo-order --no-color --pretty=""format:__START""%""n""%""h""%""n""%""p""%""n""%""D""%""n""%""ad [""%""h] {""%""an}""%""d ""%""s"" --date=iso --max-count=5000 -- "
C:\Users\daniel\scoop\apps\luajit\current\bin\luajit.exe: ...aniel\vim-config\plugged\vim-flog/lua/flog/graph_bin.lua:3: module 'flog/graph' not found:
        no field package.preload['flog/graph']
        no file '.\flog/graph.lua'
        no file 'C:\Users\daniel\scoop\apps\luajit\current\bin\..\share\luajit-2.1\flog/graph.lua'
        no file 'C:\Users\daniel\scoop\apps\luajit\current\bin\..\share\luajit-2.1\flog/graph\init.lua'
        no file 'C:\Users\daniel\scoop\apps\luajit\current\bin\..\share\lua\5.1\flog/graph.lua'
        no file 'C:\Users\daniel\scoop\apps\luajit\current\bin\..\share\lua\5.1\flog/graph\init.lua'
        no file 'C:\Users\daniel\scoop\apps\luajit\current\bin\..\lib\lua\5.1\flog/graph.lua'
        no file 'C:\Users\daniel\scoop\apps\luajit\current\bin\..\lib\lua\5.1\flog/graph\init.lua'
        no file 'C:\Users\daniel\scoop\apps\lua\current\bin'
stack traceback:
        [C]: in function 'require'
        ...aniel\vim-config\plugged\vim-flog/lua/flog/graph_bin.lua:3: in main chunk
        [C]: at 0x7ff75fca1fe0

I couldn't figure out why flog/graph is not found. If you can provide my with some guidance here, I'll try to dig dipper. However at least now it is clear that the issue comes from calling graph_bin.lua which returns the wrong output for the graph.

DanSM-5 avatar Oct 26 '24 04:10 DanSM-5

Sorry for lack of progress on this ticket. I still have to set up a pure Windows testing environment for issues like this.

The issue with the carriage returns in the output seems related with the call to systemlist() for which the help already warns about.

You're right. I had tunnel vision and thought it was in Lua, not Vim. But since it's happening with non-internal Lua it makes sense.

I pushed a fix for this issue. Let me know if it works. Might impact speed on Windows.

I couldn't figure out why flog/graph is not found

It's because path/to/vim-flog/lua/ would not automatically added to your Lua path when running something from path/to/vim-flog/lua/flog/. I think you'd need to modify the LUAPATH environment variable.

rbong avatar Oct 26 '24 15:10 rbong

I pushed a fix for this issue. Let me know if it works. Might impact speed on Windows.

I can confirm this works. As for the speed, I can't comment much since I am using a fairly powerful pc and I can't see any noticeable difference.

I think you'd need to modify the LUAPATH environment variable.

Thanks, that was it though it is called LUA_PATH. I'm finally able to run the script from the command line. I think I've found the root cause which seems to be how the command is escaped.

By command I mean the cmd argument of flog_get_graph

flog_get_graph(
    -- instance_number
    arg[1],
    -- is_vim
    false,
    -- is_nvim
    false,
    -- enable_porcelain
    true,
    -- start_token
    arg[2],
    -- enable_extended_chars
    arg[3] == 'true',
    -- enable_extra_padding
    arg[4] == 'true',
    -- enable_graph
    arg[5] == 'true',
    -- default_collapsed
    arg[6] == 'true',
    -- cmd
    arg[7],
    -- collapsed_commits
    {})

See below how the command generated by flog print the seventh argument:

Lua 5.4

> lua "C:\Users\daniel\vim-config\plugged\vim-flog/lua/flog/graph_bin.lua" 0 "__START" false false true false "git -C C:/Users/daniel/.usr_conf --git-dir C:/Users/daniel/.usr_conf/.git log --parents --topo-order --no-color --pretty=""format:__START""%""n""%""h""%""n""%""p""%""n""%""D""%""n""%""ad [""%""h] {""%""an}""%""d ""%""s"" --date=iso --max-count=5000 -- "

Arg7: git -C C:/Users/daniel/.usr_conf --git-dir C:/Users/daniel/.usr_conf/.git log --parents --topo-order --no-color --pretty="format:__START"%"n"%"h"%"n"%"p"%"n"%"D"%"n"%"ad ["%"h] {"%"an}"%"d "%"s" --date=iso --max-count=5000 --

Luajit

> luajit "C:\Users\daniel\vim-config\plugged\vim-flog/lua/flog/graph_bin.lua" 0 "__START" false false true false "git -C C:/Users/daniel/.usr_conf --git-dir C:/Users/daniel/.usr_conf/.git log --parents --topo-order --no-color --pretty=""format:__START""%""n""%""h""%""n""%""p""%""n""%""D""%""n""%""ad [""%""h] {""%""an}""%""d ""%""s"" --date=iso --max-count=5000 -- "

Arg7: git -C C:/Users/daniel/.usr_conf --git-dir C:/Users/daniel/.usr_conf/.git log --parents --topo-order --no-color --pretty="format:__START%n%h%n%p%n%D%n%ad

If I manually set the command in the lua script

flog_get_graph(
    -- instance_number
    arg[1],
    -- is_vim
    false,
    -- is_nvim
    false,
    -- enable_porcelain
    true,
    -- start_token
    arg[2],
    -- enable_extended_chars
    arg[3] == 'true',
    -- enable_extra_padding
    arg[4] == 'true',
    -- enable_graph
    arg[5] == 'true',
    -- default_collapsed
    arg[6] == 'true',
    -- cmd
    -- arg[7],
    'git -C C:/Users/daniel/.usr_conf --git-dir C:/Users/daniel/.usr_conf/.git log --parents --topo-order --no-color --pretty="format:__START"%"n"%"h"%"n"%"p"%"n"%"D"%"n"%"ad ["%"h] {"%"an}"%"d "%"s" --date=iso --max-count=5000 --',
    -- collapsed_commits
    {})

I can see the right output being printed on screen (same as displayed by lua 5.4).

The issue seems to be how luajit is escaping the input from stdin. At least it is different from how lua 5.4 is doing it which receives the input correctly.

Let me know your thoughts or if you have any ideas.

DanSM-5 avatar Oct 30 '24 00:10 DanSM-5

Let me know your thoughts or if you have any ideas.

Probably differences in how your shell is escaping strings.

I can confirm this works

Thank you, closing this issue - please let me know if there's any more problems

rbong avatar Oct 30 '24 09:10 rbong