mityu

unread,

Jul 16, 2021, 3:54:33 PM7/16/21

to vim/vim, Subscribed

I added useful setdigraphs() and getdigraphs() functions to VimScript for the following reasons.

setdigraphs()

Currently, you cannot define digraphs that start with a white space because the :digraph command cannot handle the white space on the head of chars.
Providing a function to define digraphs, in other words adding setdigraphs() function, can solve this. Example:

call setdigraphs(' a', 'あ')

Space key is really suitable for assigning to a prefix key. So, it's worth to add setdigraphs() function.

getdigraphs()

There's two reasons why I added this function.
First, if VimScript has setdigraphs() function, most users expect that getdigraphs() function exists. Having setdigraphs() function but not having getdigraphs() function is confusing.
Second, if you have getdigraphs() function, it will be easy to implement custom textobjects that have a support for digraphs like below. If you have a way to define such custom textobjects, you can make much more convenient your environment for editing texts in non-English languages on Vim. So, it will be useful.
For these reasons, I decided to implement getdigraphs() function.

function! TextobjectBetweenCharacter() abort

  let char = getcharstr()

  if char ==# "\<ESC>"

    return ''

  elseif char ==# "\<C-k>"

    let digraph = ''



    for i in range(2)

      let char = getcharstr()

      if char ==# "\<ESC>"

        return ''

      endif

      let digraph ..= char

    endfor



    let char = getdigraphs(digraph)

  endif



  " Select text between `char`...

  return ''

endfunction



omap <expr> id TextobjectBetweenCharacter()

NOTE
I'm not sure whether the function names are good. Perhaps should they be setdigraph() and getdigraph()? What do you think about this?

You can view, comment on, or merge this pull request online at:

https://github.com/vim/vim/pull/8580

Commit Summary

Add setdigraph()
Update document
Fix document
Add getdigraph()
Add #ifdef
Update document
Resolve conflict
Give error message when digraph is not supported
Declare the error messages in global.h
Add test for method
Fix build error
Fix test
Change implementation
Fix build error
Update document
Fix build error

File Changes

Patch Links:

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

mityu

unread,

Jul 16, 2021, 4:01:32 PM7/16/21

to vim/vim, Push

@mityu pushed 1 commit.

90f046f Place variable declarations on the top of blocks

—
You are receiving this because you are subscribed to this thread.

View it on GitHub or unsubscribe.

codecov[bot]

unread,

Jul 16, 2021, 4:08:40 PM7/16/21

to vim/vim, Subscribed

Codecov Report

Merging #8580 (90f046f) into master (20c370d) will decrease coverage by 87.56%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@

##           master    #8580       +/-   ##

===========================================

- Coverage   90.05%    2.49%   -87.57%     

===========================================

  Files         150      148        -2     

  Lines      168353   163130     -5223     

===========================================

- Hits       151613     4068   -147545     

- Misses      16740   159062   +142322

Flag	Coverage Δ
huge-clang-none	`?`
huge-gcc-none	`?`
huge-gcc-testgui	`?`
huge-gcc-unittests	`2.49% <0.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/digraph.c	`0.00% <0.00%> (-97.78%)`	⬇️
src/evalfunc.c	`0.00% <0.00%> (-96.25%)`	⬇️
src/ex_docmd.c	`0.00% <ø> (-95.89%)`	⬇️
src/float.c	`0.00% <0.00%> (-98.91%)`	⬇️
src/gui_gtk_f.c	`0.00% <0.00%> (-97.54%)`	⬇️
src/match.c	`0.00% <0.00%> (-97.13%)`	⬇️
src/crypt_zip.c	`0.00% <0.00%> (-97.06%)`	⬇️
src/sound.c	`0.00% <0.00%> (-97.00%)`	⬇️
src/sha256.c	`0.00% <0.00%> (-96.94%)`	⬇️
src/evalbuffer.c	`0.00% <0.00%> (-96.83%)`	⬇️
... and 139 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20c370d...90f046f. Read the comment docs.

lacygoill

unread,

Jul 16, 2021, 7:21:17 PM7/16/21

to vim/vim, Subscribed

I think all Ex commands which have a non-trivial syntax should have a function counterpart. :digraphs is one of them; it needs a lot of code to be correctly highlighted:

syntax keyword vim9DigraphsCmd dig[raphs]
    \ contained
    \ nextgroup=
    \     vim9DigraphsChars,
    \     vim9DigraphsCharsInvalid,
    \     vim9DigraphsCmdBang
    \ skipwhite
syntax match vim9DigraphsCharsInvalid /\S\+/
    \ contained
    \ nextgroup=vim9DigraphsNumber
    \ skipwhite
    syntax match vim9DigraphsCmdBang /!/ contained
syntax match vim9DigraphsChars /\s\@<=\%([^ \t|]\|\\|\)\{2}\_s\@=/
    \ contained
    \ nextgroup=vim9DigraphsNumber
    \ skipwhite
syntax match vim9DigraphsNumber /\d\+/
    \ contained
    \ nextgroup=vim9DigraphsChars,vim9DigraphsCharsInvalid
    \ skipwhite

highlight def link vim9DigraphsChars vim9String
highlight def link vim9DigraphsCmd vim9GenericCmd
highlight def link vim9DigraphsNumber vim9Number

So, I think this feature is very useful.

I'm not sure whether the function names are good. Perhaps should they be setdigraph() and getdigraph()? What do you think about this?

The command is plural, but I think that's because it can set several digraphs in a single execution.
OTOH, this function would only set 1 digraph per invocation, right? If so, yes, I think the singular form would be better.

mityu

unread,

Jul 16, 2021, 11:10:17 PM7/16/21

to vim/vim, Subscribed

I think all Ex commands which have a non-trivial syntax should have a function counterpart. :digraphs is one of them; it needs a lot of code to be correctly highlighted:

I think so. (Actually, I was going to include it in the reasons, but at that time I was so sleepy that I forgot writing it. Thank you for commenting it. I added it to my top comment :))

The command is plural, but I think that's because it can set several digraphs in a single execution.
OTOH, this function would only set 1 digraph per invocation, right? If so, yes, I think the singular form would be better.

Yes, this function sets no more than 1 digraph for one call.
I didn't know that :digrpahs command can define multiple digraphs at once, so I thought these functions names should be plural form if I follow to the command name. But now I understand why the :digraphs command is plural form and think the singular form is better too. I'll change them. Thank you.

mityu

unread,

Jul 16, 2021, 11:42:05 PM7/16/21

to vim/vim, Push

@mityu pushed 4 commits.

f2867fa Use singular form in their function name
094d0b7 Fix sometimes the error message is not correct
5241fd8 Add tests
29ca347 Fix test fails

—
You are receiving this because you are subscribed to this thread.

Yegappan Lakshmanan

unread,

Jul 17, 2021, 2:02:51 AM7/17/21

to vim/vim, Subscribed

Based on similar functions for other features, there will be feature requests
to return all the digraphs and to set multiple digraphs in a single call.

What do you think about modifying getdigraph() to return all the digraphs
if no arguments are supplied? Another approach is to introduce a new
getdigraphlist() function for this.

Similarly you can introduce a setdigraphlist() function that takes a List
of digraphs and set multiple digraphs.

mityu

unread,

Jul 17, 2021, 2:31:06 AM7/17/21

to vim/vim, Subscribed

First, by considering type-checking in vim9script, I think it's better to introduce getdigraphlist() than modifying getdigraph() to make it return all digraphs when no arguments is given. When we return one digraph, the type should be string, but when we return multiple digraphs, the type should be list<dict<string>>. Since these types are different, providing these functions separately will lead to type-checking in vim9script more strict. So, let's add getdigraphlist().

Then, in this case, it's better to add setdigraphlist() for the following reasons. First is that users will expect Vim has setdigraphlist() if Vim has getdigraphlist(). In addition to this, similar to above, introducing setdigraphlist() function can make type-checking in vim9script more strict. Therefore, I would like to take the approach that introducing setdigraphlist().

mityu

unread,

Jul 17, 2021, 3:22:48 AM7/17/21

to vim/vim, Subscribed

I'm thinking the spec of getdigraphlist() as follows:

getdigraphlist([{list-all}])

In default, it returns only the user-defined digraphs.
But if {list-all} is specified and it is true, it returns all the digraphs, including the built-in digraphs.
The type of return value is always dict.

What do you think of this, @lacygoill and @yegappan? Is it enough or should it be made more flexible?

mityu

unread,

Jul 17, 2021, 3:44:52 AM7/17/21

to vim/vim, Push

@mityu pushed 1 commit.

9933c79 Remove unnecessary #ifdef

—
You are receiving this because you are subscribed to this thread.

Bram Moolenaar

unread,

Jul 17, 2021, 10:14:53 AM7/17/21

to vim/vim, Subscribed

It indeed seems best to have four functions, set one, set list, get one, get list.

For the arguments, instead of using a number, wouldn't it be better to use a string with one character?
The big advantage is that in a script file you can read the character. For a number you would have to convert it to know what character it is.
Example: setdigraph('ho', 'ほ')
Then another advantage is that the type of the list is a list of list of strings, instead of having a mix of string and number.

mityu

unread,

Jul 17, 2021, 10:30:20 AM7/17/21

to vim/vim, Subscribed

@brammool Thank you for reviewing.

It indeed seems best to have four functions, set one, set list, get one, get list.

Now I'm thinking to use dictionary to set/get multiple digraphs because digraph is a kind of mapping. Mapping functions such as maparg(), mapcheck(), and mapset() use dictionaries. Digraph functions should follow it.
However, I don't have good idea about the name for set/get list functions. Once I named them setdigraphlist() and getdigraphlist(), but the argument/the return value is dictionary, so it's confusing. Also, I thought setdigraphitems() and getdigraphitems() are not good. Do you have any good idea for this?

For the arguments, instead of using a number, wouldn't it be better to use a string with one character?

I strongly agree. Specifying a character by number is a bit confusing. Specifying characters as-is is awesome idea. I'll update the implementation.

Yegappan Lakshmanan

unread,

Jul 17, 2021, 11:12:09 AM7/17/21

to vim_dev, reply+ACY5DGAT5KDGQSN72L...@reply.github.com, vim/vim, Subscribed

Hi,

On Sat, Jul 17, 2021 at 12:22 AM mityu <vim-dev...@256bit.org> wrote:

I'm thinking the spec of getdigraphlist() as follows:

getdigraphlist([{list-all}])

In default, it returns only the user-defined digraphs.

But if {list-all} is specified and it is true, it returns all the digraphs, including the built-in digraphs.

The type of return value is always dict.

What do you think of this, @lacygoill and @yegappan? Is it enough or should it be made more flexible?

This looks good to me.

Regards,

Yegappan

vim-dev ML

unread,

Jul 17, 2021, 11:12:28 AM7/17/21

to vim/vim, vim-dev ML, Your activity

Hi,

On Sat, Jul 17, 2021 at 12:22 AM mityu ***@***.***> wrote:

> I'm thinking the spec of getdigraphlist() as follows:
>
> getdigraphlist([{list-all}])
>

> - In default, it returns only the user-defined digraphs.
> - But if {list-all} is specified and it is true, it returns all the

> digraphs, including the built-in digraphs.

> - The type of return value is always dict.
>
> What do you think of this, @lacygoill <https://github.com/lacygoill> and
> @yegappan <https://github.com/yegappan>? Is it enough or should it be

> made more flexible?
>
>
> This looks good to me.

Regards,
Yegappan

Bram Moolenaar

unread,

Jul 17, 2021, 1:12:00 PM7/17/21

to vim/vim, vim-dev ML, Comment

> @brammool Thank you for reviewing.
>
> > It indeed seems best to have four functions, set one, set list, get one, get list.
>
> Now I'm thinking to use dictionary to set/get multiple digraphs
> because digraph is a kind of mapping. Mapping functions such as
> maparg(), mapcheck(), and mapset() use dictionaries. Digraph functions
> should follow it.
> However, I don't have good idea about the name for set/get list
> functions. Once I named them setdigraphlist() and getdigraphlist(),
> but the argument/the return value is dictionary, so it's confusing.
> Also, I thought setdigraphitems() and getdigraphitems() are not good.
> Do you have any good idea for this?

It is basically a list of pairs. That allows for duplicates, but that
can also happen by doing it with two calls.

We don't have a separate type for a pair or a tuple. Using a list with
two items works just fine. A tuple can be useful if the items have
different types, but since we now use strings only that is not relevant.

I don't see a good reason to use a dictionary. It would be possible.

--
"Hit any key to continue" it said, but nothing happened after F sharp.

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

—
You are receiving this because you commented.

mityu

unread,

Jul 17, 2021, 1:25:14 PM7/17/21

to vim/vim, vim-dev ML, Comment

Certainly. OK, I understood. Thank you. I'm going to add setdigraphlist() and getdigraphlist().

—
You are receiving this because you commented.

mityu

unread,

Jul 17, 2021, 1:47:35 PM7/17/21

to vim/vim, vim-dev ML, Push

@mityu pushed 1 commit.

01f1d8e Make it possible to specify digraph char as-is

—
You are receiving this because you are subscribed to this thread.

mityu

unread,

Jul 17, 2021, 2:19:55 PM7/17/21

to vim/vim, vim-dev ML, Push

@mityu pushed 1 commit.

4f85fd7 Implement setdigraphlist()

—
You are receiving this because you are subscribed to this thread.

mityu

unread,

Jul 17, 2021, 2:24:41 PM7/17/21

to vim/vim, vim-dev ML, Push

@mityu pushed 1 commit.

ddcd871 Fix build error

—
You are receiving this because you are subscribed to this thread.

mityu

unread,

Jul 17, 2021, 2:52:25 PM7/17/21

to vim/vim, vim-dev ML, Push

@mityu pushed 1 commit.

67241de Update document

—
You are receiving this because you are subscribed to this thread.

mityu

unread,

Jul 18, 2021, 2:24:18 PM7/18/21

to vim/vim, vim-dev ML, Push

@mityu pushed 2 commits.

b08885f Add getdigraphlist() function
75ae9b2 Add tests

—
You are receiving this because you are subscribed to this thread.

mityu

unread,

Jul 19, 2021, 9:36:35 AM7/19/21

to vim/vim, vim-dev ML, Push

@mityu pushed 2 commits.

e8c955b Add tests
11d0fc5 Update document

—
You are receiving this because you are subscribed to this thread.

mityu

unread,

Jul 19, 2021, 9:53:33 AM7/19/21

to vim/vim, vim-dev ML, Comment

Works have done.

—
You are receiving this because you commented.

mityu

unread,

Jul 19, 2021, 1:07:08 PM7/19/21

to vim/vim, vim-dev ML, Push

@mityu pushed 1 commit.

60adefa Avoid error number duplications

—
You are receiving this because you are subscribed to this thread.

mityu

unread,

Jul 19, 2021, 1:15:30 PM7/19/21

to vim/vim, vim-dev ML, Comment

Sorry, I take it back. Some problems are still left.

—
You are receiving this because you commented.

Bram Moolenaar

unread,

Jul 19, 2021, 1:31:32 PM7/19/21

to vim...@googlegroups.com, mityu

> Works have done.

Thanks. I started including it, but then found a few things.
Let me just do that instead of commenting on the pull request.
Check the patch and see if I got anything wrong.

--
hundred-and-one symptoms of being an internet addict:
183. You move your coffeemaker next to your computer.

Bram Moolenaar

unread,

Jul 19, 2021, 2:07:50 PM7/19/21

to vim/vim, vim-dev ML, Comment

Closed #8580 via 6106504.

Bram Moolenaar

unread,

Jul 19, 2021, 2:09:19 PM7/19/21

to vim...@googlegroups.com, mityu

> Sorry, I take it back. Some problems are still left.

Please sync to 8.2.3184.

--
A bad peace is better than a good war. - Yiddish Proverb

h_east

unread,

Jul 25, 2021, 10:02:55 PM7/25/21

to vim/vim, vim-dev ML, Comment

Hi Bram, mityu and all.

Most functions added since Vim 8.0 are prefixed with feature name.
For example:
Channel functions has a "ch_" prefix.
JSON functions has a "json_" prefix.
Job functions has a "job_" prefix.
Timer functions has a "timer_" prefix.
...etc.

How about renaming the digraph functions in the same way?
digraph_get(), digraph_get_list(), digraph_set(), digraph_set_list().

Sorry for the late proposal🙇.

--
Best regards,
Hirohito Higashi (h_east)

mityu

unread,

Jul 26, 2021, 10:44:33 AM7/26/21

to vim/vim, vim-dev ML, Comment

Certainly, sign related functions also have sign_ prefix. It looks good to me.

Bram Moolenaar

unread,

Jul 26, 2021, 3:39:28 PM7/26/21

to vim/vim, vim-dev ML, Comment

Yes, but then only one underscore: digraph_getlist(), digraph_setlist().

Reply all

Reply to author

Forward

[vim/vim] Add setdigraphs() and getdigraphs() functions (#8580)

mityu

You can view, comment on, or merge this pull request online at:

Commit Summary

File Changes

Patch Links:

mityu

codecov[bot]

Codecov Report

lacygoill

mityu

mityu

Yegappan Lakshmanan

mityu

mityu

mityu

Bram Moolenaar

mityu

Yegappan Lakshmanan

vim-dev ML

Bram Moolenaar

mityu

mityu

mityu

mityu

mityu

mityu

mityu

mityu

mityu

mityu

Bram Moolenaar

Bram Moolenaar

Bram Moolenaar

h_east

mityu

Bram Moolenaar