[vim/vim] add small inner trim function (#1280)
Bukn
I add a small inner function just like php's trim or python's strip.
If merged, in vim script, we would be able to use
let str = trim(" some_string_with_spaces_quoted \t\t ")
to trim the spaces. And str would be "some_string_with_spaces_quoted"
If we want to trim other characters, just add the second parameter.
let str = trim("xxxyyyJUST NEED THISzzzwww", "wxyz")
We would got s = "JUST NEED THIS"
I'm a 7-yeared vim user. This is my first commit. Thank you for you guys built such a great editor.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/1280
Commit Summary
- add small inner trim function
File Changes
- M src/evalfunc.c (52)
Patch Links:
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
Justin M. Keyes
Needs tests. https://github.com/vim/vim/blob/master/src/testdir/README.txt
I am often annoyed that trim() does not exist. It can be achieved with substitute(), if one is careful, but it's still annoying. So I would welcome this change.
mattn
Needs tests.
and doc.
mattn
This trim doesn't support multi-byte.
Bukn
Test added. But simple.
Can someone give me a few instructions on how to add docs?
And I will work on multibyte support
Naruhiko Nishino
Can someone give me a few instructions on how to add docs?
https://github.com/vim/vim/pull/1277/files
Okey. Please refer to changes of This PR.
Naruhiko Nishino
And I will work on multibyte support
Please show me your Vim script codes working on multibyte support and you should add multibyte test.
mattn
Justin M. Keyes
This should have at least 30 different test cases. There are many possible input variations.
mattn
Oops, my patch is wrong. will fix soon.
mattn
fixed my patch. feel free to use.
Bukn
How can I merge mattn's patch ? I'm still thinking on how to test multibyte. Maybe test Chinese characters, since I'm from China.
mattn
mattn
Sorry again. please wait merging. my patch still wrong.
mattn
Should be fixed.
Bukn
@bukn pushed 12 commits.
- 2645282
- 7b16a5a
- bc24a24
- 582d608
- fce4d11
- 33f9b0f
- 6b65fa9
- ec511c1 testing makefile entrace
- 75abf69 Merge branch 'master' of https://github.com/vim/vim into inner_trim_func
- 3aa2f95 Merge https://gist.github.com/mattn/3bc46f6618c7d170dec403d8c05be7fb into inner_trim_func
- b0d64c7 new test for trim
- f03f71e replace trim logic with mattn's code which supported multibytes; test cases has multibytes character(Chinese)
—
You are receiving this because you are subscribed to this thread.
Bukn
Hope the multibytes test cases are enough.
Bukn
Doc added~
mattn
please add more three tests.
trim("", "")trim("", "a")trim("a", "")
K.Takata
@k-takata commented on this pull request.
> @@ -2824,8 +2826,8 @@ char2nr({expr}[, {utf8}]) *char2nr()*
char2nr("ABC") returns 65
< When {utf8} is omitted or zero, the current 'encoding' is used.
Example for "utf-8": >
- char2nr("�") returns 225
- char2nr("�"[0]) returns 195
+ char2nr("?") returns 225
+ char2nr("?"[0]) returns 195
This change is not needed.
It's better to set enc=utf-8 when you edit eval.txt.
mattn
@mattn commented on this pull request.
> @@ -2824,8 +2826,8 @@ char2nr({expr}[, {utf8}]) *char2nr()*
char2nr("ABC") returns 65
< When {utf8} is omitted or zero, the current 'encoding' is used.
Example for "utf-8": >
- char2nr("�") returns 225
- char2nr("�"[0]) returns 195
+ char2nr("?") returns 225
+ char2nr("?"[0]) returns 195
should be latin-1. :e ++enc=latin-1 runtime/doc/eval.txt
Bukn
@bukn commented on this pull request.
> @@ -2824,8 +2826,8 @@ char2nr({expr}[, {utf8}]) *char2nr()*
char2nr("ABC") returns 65
< When {utf8} is omitted or zero, the current 'encoding' is used.
Example for "utf-8": >
- char2nr("�") returns 225
- char2nr("�"[0]) returns 195
+ char2nr("?") returns 225
+ char2nr("?"[0]) returns 195
fixed
—
You are receiving this because you are subscribed to this thread.
Bukn
How to pass the first check? It seemed that not this pr's problem.
K.Takata
How to pass the first check?
Do you mean coveralls?
You don't need to worry about it if your test covers the newly written part. (Sometimes coveralls reports unexpected result.)
However seeing the result, f_trim() doesn't seem to be executed. Something missing with your test?
Ismael Santos Kafeltz
I'm just curious why this function didn't exists all this vim lifetime so far.
LCD 047
I'm just curious why this function didn't exists all this vim lifetime so far.
Because it isn't that hard to get the same effect without it:
let trimmed = substitute('xxxyyyJUST NEED THISzzzwww', '\m^[wxyz]*\(.\{-}\)[wxyz]*$', '\1', '')
or
let trimmed = matchstr('xxxyyyJUST NEED THISzzzwww', '\m^[wxyz]*\zs.\{-}\ze[wxyz]*$')
mattn
As you know, it's difficult to use substitute in map().
:echo map(list, "sustitute(v:val, '\\m^[wxyz]*\\zs.\{-}\\ze[wxyz]*$')")
but trim is
:echo map(list, "trim(v:val, 'wxyz')")
Christian Brabandt
well, lambda expressions make it readable again. Also, while I agree that the pattern becomes hard to read, you could always store the pattern in a separate variable and use that one explicitly in the map() call.
That doesn't mean, I am against a trim() function
LCD 047
As you know, it's difficult to use substitute in map().
Sure, then you can do something like this:
function! Trim(what, junk) let rx = '[' . substitute(a:junk, '\m\([]^\\-]\)', '\\\1', 'g') . ']*' return matchstr(a:what, '\m^' . rx . '\zs.\{-}\ze' . rx . '$') endfunction let trimmed_list = map(list, "Trim(v:val, 'wxyz')")
Which isn't to say I'm against adding a trim() function, either. Just pointing out that missing it until now wasn't a crippling issue. There are other functions that are much harder to replace.
Bukn
However seeing the result, f_trim() doesn't seem to be executed. Something missing with your test?
I added test followed this instruction https://github.com/vim/vim/blob/master/src/testdir/README.txt
It says:
2) Add test_.vim to NEW_TESTS in Make_all.mak in alphabetical order.
But I saw that in Make_all.mak, NEW_TESTS section, the cases are all named with .res postfix. Is this the problem? And is it a little mistake of the document ?
Bukn
Bram Moolenaar
Bukn wrote:
> > However seeing the result, f_trim() doesn't seem to be executed. Something missing with your test?
>
> I added test followed this instruction https://github.com/vim/vim/blob/master/src/testdir/README.txt
> It says:
>
>
> But I saw that in Make_all.mak, NEW_TESTS section, the cases are all
> named with .res postfix. Is this the problem? And is it a little
> mistake of the document ?
--
hundred-and-one symptoms of being an internet addict:
59. Your wife says communication is important in a marriage...so you buy
another computer and install a second phone line so the two of you can
chat.
/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
—
You are receiving this because you are subscribed to this thread.
Bukn
Ah, is there anything I need to improve? How to know vim's new feature merge plan?
—
You are receiving this because you are subscribed to this thread.
K.Takata
Your patch has been already added in the todo.txt.
https://github.com/vim/vim/blob/3421566376b5723213af502bd3c2b9debe025ef1/runtime/doc/todo.txt#L192
So just wait patiently. Maybe a few weeks or a few months, (or sometimes even a few years).
Adding new feature has lower priority than fixing bugs.
How to know vim's new feature merge plan?
Only Bram knows. 😉