Trailing CR when using omni completion on Windows

[syringus]

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:

This behavior is caused by Windows using CRLF as new line characters and systemlist() function splitting lines on LF character:

vim-ledger/ftplugin/ledger.vim

Lines 399 to 404 in 281346a

	function! s:collect_completion_data() "{{{1
	let transactions = ledger#transactions()
	let cache = {'descriptions': [], 'tags': {}, 'accounts': {}}
	if exists('g:ledger_accounts_cmd')
	let accounts = systemlist(g:ledger_accounts_cmd)
	else

My quick solution is to remove CR characters from the strings in accounts list:

vim-ledger/ftplugin/ledger.vim

Lines 402 to 406 in 4fc1366

	if exists('g:ledger_accounts_cmd')
	let accounts = systemlist(g:ledger_accounts_cmd)
	" Remove trailing CR characters on Windows (see note in systemlist() help).
	let accounts = map(accounts, 'substitute(v:val, "\\r$", "", "")')
	else

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

Best regards,
Martin.

[alerque]

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]

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.