ledger / vim-ledger

Vim plugin for Ledger
GNU General Public License v2.0
370 stars 56 forks source link

Trailing CR when using omni completion on Windows #151

Closed syringus closed 4 months ago

syringus commented 8 months ago

Hello,

I face an issue with trailing CR characters when using omni completion for account names in neovim on Windows. After pressing CTRL+x, o, the account names suggestions are correct, but there is a trailing CR character at the end: image

This behavior is caused by Windows using CRLF as new line characters and systemlist() function splitting lines on LF character: https://github.com/ledger/vim-ledger/blob/281346a221434574dd7f8767a352b2bf0b218b74/ftplugin/ledger.vim#L399-L404

My quick solution is to remove CR characters from the strings in accounts list: https://github.com/ledger/vim-ledger/blob/4fc136660e404c5acbedcc6b092ba5b2619abed4/ftplugin/ledger.vim#L402-L406

I have no experience in vimscript, so the solution is far from optimal.

Best regards, Martin.

alerque commented 8 months ago

Thanks for the report. I'm not on Windows myself and so won't be able to test this much. Your solution looks a bit like a hack, but I can't think of anything better necessarily without knowing if there is some built in function to handle this. Like perhaps systemlist() isn't what we want, or perhaps it has arguments for handling this.

In any case, the only thig I would do for now is gate this in an if block behind as OS test so that it doesn't have to run on all systems. If you'd like to work that up as a PR I'd say we can get started with that until such a time as somebody has a better solution to suggest.

syringus commented 8 months ago

I agree that my quick solution is a bit hackish.

I looked for an alternative to systemlist() and it seems that using system() and creating a list from the returned string is a better approach. According to system() documentation:

Result is a String, filtered to avoid platform-specific quirks:

  • CRNL is replaced with NL

I will create a PR for this issue replacing all systemlist() calls with split(system(), '\n').

Thanks.