[PATCH 1/2] doc/autocmd.txt: Add missing events in *autocmd-events*.

Simon Ruderich

unread,

Sep 4, 2012, 11:19:56 AM9/4/12

to vim...@googlegroups.com

Simon Ruderich

unread,

Sep 4, 2012, 11:20:24 AM9/4/12

to vim...@googlegroups.com

-|QuitPre| when using `:quit`, before deciding whether to quit

+|QuitPre| when using `:quit`, before deciding whether to quit

|VimLeavePre| before exiting Vim, before writing the viminfo file
|VimLeave| before exiting Vim, after writing the viminfo file

|MenuPopup| just before showing the popup menu

|CompleteDone| after inster mode completion is done

Taylor Hedberg

unread,

Sep 4, 2012, 11:23:50 AM9/4/12

to vim...@googlegroups.com

Simon Ruderich, Tue 2012-09-04 @ 17:19:56+0200:

Small typo: s/inster/insert/

signature.asc

Dominique Pellé

unread,

Sep 4, 2012, 12:14:19 PM9/4/12

to vim...@googlegroups.com

Simon Ruderichwrote:

Hi Simon

I can see why you want to patch this, but I think that the patch should
not be applied. It's a work around for a bug in Vim. It's also breaking
indentation when using "set concealllevel=0" or when conceal feature
is not enabled at compilation time.

It's the same kind of patch that was already rejected. See:

https://groups.google.com/forum/?fromgroups=#!topic/vim_dev/QowVzxfhgbM

There is a patch pending which fixes those alignment issues among other
problems (whether Vim uses conceal feature or not).

The proposed patch has been available for a long time by the way
without having been integrated in the official Mercurial repository.
I understand that tests were requested for it, but I don't really see
how it's possible to test it. Any chance to get it integrated anyway?
I've been using it for a long time without problem.

Regards
-- Dominique

Simon Ruderich

unread,

Sep 4, 2012, 6:16:03 PM9/4/12

to vim...@googlegroups.com

On Tue, Sep 04, 2012 at 06:14:19PM +0200, Dominique Pellé wrote:
> Hi Simon
>
> I can see why you want to patch this, but I think that the patch should
> not be applied. It's a work around for a bug in Vim. It's also breaking
> indentation when using "set concealllevel=0" or when conceal feature
> is not enabled at compilation time.
>
> It's the same kind of patch that was already rejected. See:
>
> https://groups.google.com/forum/?fromgroups=#!topic/vim_dev/QowVzxfhgbM

Hello Dominique,

Thank you, I didn't know that. I saw a few similar changes in the
past so I thought it was fine.

> There is a patch pending which fixes those alignment issues among other
> problems (whether Vim uses conceal feature or not).

Thanks for pointing me to this patch. It fixes a few tab related
issues I just noticed (and tried to fix - you saved me some time,
thanks).

> The proposed patch has been available for a long time by the way
> without having been integrated in the official Mercurial repository.
> I understand that tests were requested for it, but I don't really see
> how it's possible to test it. Any chance to get it integrated anyway?
> I've been using it for a long time without problem.

I'd really like to get it integrated too, it fixed the issues I
was seeing.

It took me a while, but I created a testcase which checks the
output of Vim running with TERM=builting_dumb and it works fine
for me, but better solutions are welcome.

I'm not sure what's the best way to include it in Vim's testsuite
because, please help me with that part. At the moment I just
added it as test88.sh (see the patch) and updated the Makefile to
run it.

Regards,
Simon

0001-testdir-Add-test88-to-check-conceal-with-tabulators.patch

Simon Ruderich

unread,

Sep 4, 2012, 6:20:37 PM9/4/12

to vim...@googlegroups.com

> --- /dev/null
> +++ b/src/testdir/test88.ok
> @@ -0,0 +1,34 @@
>
> [snip]
>
> +[8;1H
> +[8;9H
> +[8;17H
> +[8;25H
> +[8;26H
> +[24;1H :q <-
> +[24;1H <-

Hm, looks like the patch was mangled. In both marked spots (<-)
there's a ^M (\r) missing at the end-of-line.

Bram Moolenaar

unread,

Sep 5, 2012, 9:46:00 AM9/5/12

to Simon Ruderich, vim...@googlegroups.com

Simon Ruderich wrote:

[...]

> > There is a patch pending which fixes those alignment issues among other
> > problems (whether Vim uses conceal feature or not).
>
> Thanks for pointing me to this patch. It fixes a few tab related
> issues I just noticed (and tried to fix - you saved me some time,
> thanks).

Which patch is that, who sent it?

> > The proposed patch has been available for a long time by the way
> > without having been integrated in the official Mercurial repository.
> > I understand that tests were requested for it, but I don't really see
> > how it's possible to test it. Any chance to get it integrated anyway?
> > I've been using it for a long time without problem.
>
> I'd really like to get it integrated too, it fixed the issues I
> was seeing.
>
> It took me a while, but I created a testcase which checks the
> output of Vim running with TERM=builting_dumb and it works fine
> for me, but better solutions are welcome.
>
> I'm not sure what's the best way to include it in Vim's testsuite
> because, please help me with that part. At the moment I just
> added it as test88.sh (see the patch) and updated the Makefile to
> run it.

This doesn't look like a nice solution. We could add something in Vim
to report the screen position of the cursor. Perhaps with a function.
The test could use that to check where the cursor really is.

--
Nothing is impossible for the man who doesn't have to do it.

/// 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 ///

Bram Moolenaar

unread,

Sep 5, 2012, 9:45:59 AM9/5/12

to Simon Ruderich, vim...@googlegroups.com

I'll include it, thanks.

--
GUARD #1: What -- a swallow carrying a coconut?
ARTHUR: It could grip it by the husk!
GUARD #1: It's not a question of where he grips it! It's a simple question
of weight ratios! A five ounce bird could not carry a 1 pound
coconut.
The Quest for the Holy Grail (Monty Python)

Bram Moolenaar

unread,

Sep 5, 2012, 9:45:59 AM9/5/12

to Simon Ruderich, vim...@googlegroups.com

Simon Ruderich wrote:

> diff --git a/runtime/doc/autocmd.txt b/runtime/doc/autocmd.txt
> index 7a5fa04..fe77265 100644
> --- a/runtime/doc/autocmd.txt
> +++ b/runtime/doc/autocmd.txt
> @@ -203,7 +203,7 @@ Name triggered by ~
> Reading
> |BufNewFile| starting to edit a file that doesn't exist
> |BufReadPre| starting to edit a new buffer, before reading the file
> -|BufRead| starting to edit a new buffer, after reading the file
> +|BufRead| starting to edit a new buffer, after reading the file
> |BufReadPost| starting to edit a new buffer, after reading the file
> |BufReadCmd| before starting to edit a new buffer |Cmd-event|

[...]

Thanks, but I think we need to fix the alignment problem first. The way
conceal handles tabs just doesn't work well.

--
"A mouse can be just as dangerous as a bullet or a bomb."
(US Representative Lamar Smith, R-Texas)

Simon Ruderich

unread,

Sep 5, 2012, 12:59:15 PM9/5/12

to vim...@googlegroups.com

On Wed, Sep 05, 2012 at 03:46:00PM +0200, Bram Moolenaar wrote:
>> Thanks for pointing me to this patch. It fixes a few tab related
>> issues I just noticed (and tried to fix - you saved me some time,
>> thanks).
>
> Which patch is that, who sent it?

It's Dominique Pellé's patch:
http://comments.gmane.org/gmane.editors.vim.devel/33830

> This doesn't look like a nice solution. We could add something in Vim
> to report the screen position of the cursor. Perhaps with a function.
> The test could use that to check where the cursor really is.

You're right it isn't. A function would be much better. I don't
know the Vim internals very well, so I just used what was already
available.

I tried to add the functions (patch attached - as I'm new to Vim
internals, please tell me if that's the correct way to do it),
but there is one major problem. screen_cur_col and screen_cur_row
report the current cursor position - which is of course the
command line where the current command is entered (also in
tests), e.g. 0, 38.

That means we have to save the current cursor position before
jumping to the command line when the users enters a command, but
I don't know what's the correct place for this task. Please help
me with that.

0001-Add-screencol-and-screenrow-functions.patch

ZyX

unread,

Sep 5, 2012, 1:22:34 PM9/5/12

to vim...@googlegroups.com, si...@ruderich.org

> I tried to add the functions (patch attached - as I'm new to Vim
> internals, please tell me if that's the correct way to do it),
> but there is one major problem. screen_cur_col and screen_cur_row
> report the current cursor position - which is of course the
> command line where the current command is entered (also in
> tests), e.g. 0, 38.
>
> That means we have to save the current cursor position before
> jumping to the command line when the users enters a command, but
> I don't know what's the correct place for this task. Please help
> me with that.

It works from inside expression mappings, like
nnoremap <expr> GG ":echom ".screenrow()." ".screencol()."\n"
. I believe it is fine and correct behavior: always reporting real cursor position. Not very convenient, but correct, and completely enough for tests. Just mention this workaround in the doc.

But it was much better not to have “screen(col/row)()” with no arguments which report about cursor position, but something like “virtcol()”: with ability to report screen position of arbitrary byte position. If I knew, how to do it it would be just optional second argument to “virtcol()” that, if present and non-empty, makes it take concealed characters into account. I am seeing at least one problem with this: with this argument “virtcol()” will be forced to push syntax rules application once called for invisible position, or it will report invalid results. There must be more.

Charles Campbell

unread,

Sep 5, 2012, 4:21:08 PM9/5/12

to vim...@googlegroups.com

Simon Ruderich wrote:
> On Wed, Sep 05, 2012 at 03:46:00PM +0200, Bram Moolenaar wrote:
>
>>> Thanks for pointing me to this patch. It fixes a few tab related
>>> issues I just noticed (and tried to fix - you saved me some time,
>>> thanks).
>>>
>> Which patch is that, who sent it?
>>

> It's Dominique Pell�'s patch:
> http://comments.gmane.org/gmane.editors.vim.devel/33830
>

Unfortunately, when I attempt to get the patch via gmane, I get some
unusable binary file instead of the patch.

Would someone please re-post it?

Thanks,
Chip Campbell

Charles Campbell

unread,

Sep 5, 2012, 4:26:47 PM9/5/12

to vim...@googlegroups.com

Please disregard this request. Got it!

Regards,
Chip Campbell

Simon Ruderich

unread,

Sep 6, 2012, 5:27:12 AM9/6/12

to vim...@googlegroups.com

On Wed, Sep 05, 2012 at 10:22:34AM -0700, ZyX wrote:
> It works from inside expression mappings, like
> nnoremap <expr> GG ":echom ".screenrow()." ".screencol()."\n"
> . I believe it is fine and correct behavior: always reporting
> real cursor position. Not very convenient, but correct, and
> completely enough for tests. Just mention this workaround in
> the doc.

Hello ZyX,

Thanks for the hint. Attached is a testcase which ports my old
tests to use your suggestion.

I noticed that screencol()/screenrow() start from zero while the
other *col() functions start from one. Should I update my
screencol()/screenrow() patch?

Regards,
Simon

---
src/testdir/Makefile | 4 +-
src/testdir/test88.in | 84 +++++++++++++++++++++++++++++++++++++++++++++++++
src/testdir/test88.ok | 23 +++++++++++++
3 files changed, 109 insertions(+), 2 deletions(-)
create mode 100644 src/testdir/test88.in
create mode 100644 src/testdir/test88.ok

diff --git a/src/testdir/Makefile b/src/testdir/Makefile
index 20d9781..936bca9 100644
--- a/src/testdir/Makefile
+++ b/src/testdir/Makefile
@@ -13,7 +13,7 @@ VIMPROG = ../vim

SCRIPTS = test1.out test2.out test3.out test4.out test5.out test6.out \
test7.out test8.out test9.out test10.out test11.out \
- test12.out test13.out test14.out test15.out test17.out \
+ test12.out test13.out test14.out test15.out test17.out \
test18.out test19.out test20.out test21.out test22.out \
test23.out test24.out test25.out test26.out test27.out \
test28.out test29.out test30.out test31.out test32.out \
@@ -27,7 +27,7 @@ SCRIPTS = test1.out test2.out test3.out test4.out test5.out test6.out \
test69.out test70.out test71.out test72.out test73.out \
test74.out test75.out test76.out test77.out test78.out \
test79.out test80.out test81.out test82.out test83.out \
- test84.out test85.out test86.out test87.out
+ test84.out test85.out test86.out test87.out test88.out \

SCRIPTS_GUI = test16.out

diff --git a/src/testdir/test88.in b/src/testdir/test88.in
new file mode 100644
index 0000000..26b9a1d
--- /dev/null
+++ b/src/testdir/test88.in
@@ -0,0 +1,84 @@
+vim: set ft=vim
+
+Tests for correct display (cursor column position) with +conceal and
+tabulators.
+
+STARTTEST
+:so small.vim
+:if !has('conceal')
+ e! test.ok
+ wq! test.out
+:endif
+:" Conceal settings.
+:set conceallevel=2
+:set concealcursor=nc
+:syntax match test /|/ conceal
+:" Save current cursor position. Only works in <expr> mode, can't be used
+:" with :normal because it moves the cursor to the command line.
+:let positions = []
+:nnoremap <expr> GG ":let positions += ['".screenrow().":".screencol()."']\n"
+:" Start test.
+/^start:
+:normal ztj
+GGk
+:" We should end up in the same column when running these commands on the two
+:" lines.
+:normal ft
+GGk
+:normal $
+GGk
+:normal 0j
+GGk
+:normal ft
+GGk
+:normal $
+GGk
+:normal 0j0j
+GGk
+:" Same for next test block.
+:normal ft
+GGk
+:normal $
+GGk
+:normal 0j
+GGk
+:normal ft
+GGk
+:normal $
+GGk
+:normal 0j0j
+GGk
+:" And check W with multiple tabs and conceals in a line.
+:normal W
+GGk
+:normal W
+GGk
+:normal W
+GGk
+:normal $
+GGk
+:normal 0j
+GGk
+:normal W
+GGk
+:normal W
+GGk
+:normal W
+GGk
+:normal $
+GGk
+:" Display result.
+:call append('$', 'end:')
+:call append('$', positions)
+:/^end/,$wq! test.out
+ENDTEST
+
+start:
+.concealed. text
+|concealed| text
+
+ .concealed. text
+ |concealed| text
+
+.a. .b. .c. .d.
+|a| |b| |c| |d|
diff --git a/src/testdir/test88.ok b/src/testdir/test88.ok
new file mode 100644
index 0000000..b9f798e
--- /dev/null
+++ b/src/testdir/test88.ok
@@ -0,0 +1,23 @@
+end:
+1:0
+1:16
+1:19
+2:0
+2:16
+2:19
+4:7
+4:24
+4:27
+5:7
+5:24
+5:27
+7:0
+7:8
+7:16
+7:24
+7:26
+8:0
+8:8
+8:16
+8:24
+8:25
--
1.7.7.3

Bram Moolenaar

unread,

Sep 6, 2012, 4:36:46 PM9/6/12

to Simon Ruderich, vim...@googlegroups.com

Simon Ruderich wrote:

> Thanks for the hint. Attached is a testcase which ports my old
> tests to use your suggestion.
>
> I noticed that screencol()/screenrow() start from zero while the
> other *col() functions start from one. Should I update my
> screencol()/screenrow() patch?

Yes, it would be nice to keep this consistent. virtcol() also returns 1
for the first column.

--
ARTHUR: I did say sorry about the `old woman,' but from the behind you
looked--
DENNIS: What I object to is you automatically treat me like an inferior!
ARTHUR: Well, I AM king...

The Quest for the Holy Grail (Monty Python)

Simon Ruderich

unread,

Sep 6, 2012, 7:40:52 PM9/6/12

to vim...@googlegroups.com

On Thu, Sep 06, 2012 at 10:36:46PM +0200, Bram Moolenaar wrote:
> Yes, it would be nice to keep this consistent. virtcol() also returns 1
> for the first column.

Updated patch with adapted screencol()/screenrow(), updated
testcase (needs Dominique's patch applied) and documentation
attached.

I hope that's enough to get Dominique's patch into Vim.

Regards,
Simon
---
runtime/doc/eval.txt | 18 ++++++++++
src/eval.c | 26 +++++++++++++++
src/screen.c | 19 +++++++++++
src/testdir/Makefile | 4 +-
src/testdir/test88.in | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
src/testdir/test88.ok | 23 +++++++++++++
6 files changed, 173 insertions(+), 2 deletions(-)

create mode 100644 src/testdir/test88.in
create mode 100644 src/testdir/test88.ok

diff --git a/runtime/doc/eval.txt b/runtime/doc/eval.txt
index 2ae65ff..4763fd0 100644
--- a/runtime/doc/eval.txt
+++ b/runtime/doc/eval.txt
@@ -1903,6 +1903,8 @@ repeat( {expr}, {count}) String repeat {expr} {count} times
resolve( {filename}) String get filename a shortcut points to
reverse( {list}) List reverse {list} in-place
round( {expr}) Float round off {expr}
+screencol() Number current cursor column
+screenrow() Number current cursor row
search( {pattern} [, {flags} [, {stopline} [, {timeout}]]])
Number search for {pattern}
searchdecl( {name} [, {global} [, {thisblock}]])
@@ -4867,6 +4869,22 @@ round({expr}) *round()*
< -5.0
{only available when compiled with the |+float| feature}

+screencol() *screencol()*
+ The result is a Number, which is the current screen column of
+ the cursor. This function is mainly used for testing.
+
+ Note: Always returns the current screen column, thus if used
+ in a command (e.g. ":echo screencol()") it will return the
+ column inside the command line, which is 1 when the command is
+ executed. To get the cursor position in the file use a
+ mapping: >
+ nnoremap <expr> GG ":echom ".screencol()."\n"
+<
+screenrow() *screenrow()*
+ The result is a Number, which is the current screen row of the
+ the cursor. This function is mainly used for testing.
+
+ Note: Same restrictions as |screencol()|.

search({pattern} [, {flags} [, {stopline} [, {timeout}]]]) *search()*
Search for regexp pattern {pattern}. The search starts at the
diff --git a/src/eval.c b/src/eval.c
index 98b2a33..5bdefda 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -668,6 +668,8 @@ static void f_reverse __ARGS((typval_T *argvars, typval_T *rettv));
#ifdef FEAT_FLOAT
static void f_round __ARGS((typval_T *argvars, typval_T *rettv));
#endif
+static void f_screencol __ARGS((typval_T *argvars, typval_T *rettv));
+static void f_screenrow __ARGS((typval_T *argvars, typval_T *rettv));
static void f_search __ARGS((typval_T *argvars, typval_T *rettv));
static void f_searchdecl __ARGS((typval_T *argvars, typval_T *rettv));
static void f_searchpair __ARGS((typval_T *argvars, typval_T *rettv));
@@ -8032,6 +8034,8 @@ static struct fst
#ifdef FEAT_FLOAT
{"round", 1, 1, f_round},
#endif
+ {"screencol", 0, 0, f_screencol},
+ {"screenrow", 0, 0, f_screenrow},
{"search", 1, 4, f_search},
{"searchdecl", 1, 3, f_searchdecl},
{"searchpair", 3, 7, f_searchpair},
@@ -15717,6 +15721,28 @@ f_round(argvars, rettv)
#endif

/*
+ * "screencol()" function
+ */
+ static void
+f_screencol(argvars, rettv)
+ typval_T *argvars;
+ typval_T *rettv;
+{
+ rettv->vval.v_number = screen_screencol() + 1;
+}
+
+/*
+ * "screenrow()" function
+ */
+ static void
+f_screenrow(argvars, rettv)
+ typval_T *argvars;
+ typval_T *rettv;
+{
+ rettv->vval.v_number = screen_screenrow() + 1;
+}
+
+/*
* "search()" function
*/
static void
diff --git a/src/screen.c b/src/screen.c
index 5dccbd7..6892203 100644
--- a/src/screen.c
+++ b/src/screen.c
@@ -10263,3 +10263,22 @@ number_width(wp)
return n;
}
#endif
+
+/*
+ * Return the current cursor column. This is the actual position on the
+ * screen.
+ */
+ int
+screen_screencol()
+{
+ return screen_cur_col;
+}
+
+/*
+ * Return the current cursor row. This is the actual position on the screen.
+ */
+ int
+screen_screenrow()
+{
+ return screen_cur_row;
+}
diff --git a/src/testdir/Makefile b/src/testdir/Makefile
index dfcc8c0..7403abc 100644

--- a/src/testdir/Makefile
+++ b/src/testdir/Makefile
@@ -13,7 +13,7 @@ VIMPROG = ../vim

SCRIPTS = test1.out test2.out test3.out test4.out test5.out test6.out \
test7.out test8.out test9.out test10.out test11.out \
- test12.out test13.out test14.out test15.out test17.out \
+ test12.out test13.out test14.out test15.out test17.out \
test18.out test19.out test20.out test21.out test22.out \
test23.out test24.out test25.out test26.out test27.out \
test28.out test29.out test30.out test31.out test32.out \
@@ -27,7 +27,7 @@ SCRIPTS = test1.out test2.out test3.out test4.out test5.out test6.out \
test69.out test70.out test71.out test72.out test73.out \
test74.out test75.out test76.out test77.out test78.out \
test79.out test80.out test81.out test82.out test83.out \
- test84.out test85.out test86.out test87.out
+ test84.out test85.out test86.out test87.out test88.out \

SCRIPTS_GUI = test16.out

diff --git a/src/testdir/test88.in b/src/testdir/test88.in
new file mode 100644

index 0000000..1536b3b
--- /dev/null
+++ b/src/testdir/test88.in
@@ -0,0 +1,85 @@

+vim: set ft=vim
+
+Tests for correct display (cursor column position) with +conceal and
+tabulators.
+
+STARTTEST
+:so small.vim
+:if !has('conceal')
+ e! test.ok
+ wq! test.out
+:endif
+:" Conceal settings.
+:set conceallevel=2
+:set concealcursor=nc
+:syntax match test /|/ conceal
+:" Save current cursor position. Only works in <expr> mode, can't be used

+:" with :normal because it moves the cursor to the command line. Thanks to ZyX
+:" <zyx...@gmail.com> for the idea to use an <expr> mapping.

index 0000000..e726258

--- /dev/null
+++ b/src/testdir/test88.ok
@@ -0,0 +1,23 @@
+end:

+2:1
+2:17
+2:20
+3:1
+3:17
+3:20
+5:8
+5:25
+5:28
+6:8
+6:25
+6:28
+8:1
+8:9
+8:17
+8:25
+8:27
+9:1
+9:9
+9:17
+9:25
+9:26

Dominique Pellé

unread,

Sep 6, 2012, 10:20:40 PM9/6/12

to vim...@googlegroups.com

Simon Ruderich wrote:

> On Thu, Sep 06, 2012 at 10:36:46PM +0200, Bram Moolenaar wrote:
>> Yes, it would be nice to keep this consistent. virtcol() also returns 1
>> for the first column.
>
> Updated patch with adapted screencol()/screenrow(), updated
> testcase (needs Dominique's patch applied) and documentation
> attached.
>
> I hope that's enough to get Dominique's patch into Vim.

Thanks Simon for the patch! Some remarks:

- It's better to send the patch as attachment because long
lines were messed up by mail somehow.

- I manually fixed the long lines in patch and it applied.
But I saw a warning:

Hunk #1 succeeded at 1902 with fuzz 2 (offset -1 lines).

The patch was probably not made against the latest Vim
in Mercurial.

- declaration of screen_screencol() and screen_screenrow()
were missing in src/proto/screen.c.pro, so when compiling,
I saw warnings:

screen.c:10260:1: warning: no previous prototype for
‘screen_screencol’ [-Wmissing-prototypes]
screen.c:10269:1: warning: no previous prototype for
‘screen_screenrow’ [-Wmissing-prototypes]

- in ":help screenrow()" I noticed a repeated word in:
"screen row of *the the* cursor

I attach a patch against Vim-7.3.659 which:

- addresses all above minor issues
- combine my original patch with your tests in one single
patch for convenience.

Regards

PS: worth noticing that instead of...

nnoremap <expr> GG ":echom ".screencol()."\n"

... we can also use:

nnoremap <silent> GG :echom screencol()<CR>

-- Dominique

fix-conceal-alignment-7.3.659.patch

Simon Ruderich

unread,

Sep 7, 2012, 9:51:49 AM9/7/12

to vim...@googlegroups.com

On Fri, Sep 07, 2012 at 04:20:40AM +0200, Dominique Pellé wrote:
> Thanks Simon for the patch! Some remarks:

Hello Dominique,

> - It's better to send the patch as attachment because long
> lines were messed up by mail somehow.

Thanks for the suggestion, will do it in the future. It's weird
though, plain text patches work fine for other projects.

> - I manually fixed the long lines in patch and it applied.
> But I saw a warning:
>
> Hunk #1 succeeded at 1902 with fuzz 2 (offset -1 lines).
>
> The patch was probably not made against the latest Vim
> in Mercurial.

Maybe a few of my local commits interfered, sorry for that.

> - declaration of screen_screencol() and screen_screenrow()
> were missing in src/proto/screen.c.pro, so when compiling,
> I saw warnings:
>
> screen.c:10260:1: warning: no previous prototype for
> ‘screen_screencol’ [-Wmissing-prototypes]
> screen.c:10269:1: warning: no previous prototype for
> ‘screen_screenrow’ [-Wmissing-prototypes]

Thank you, somehow I didn't see those warnings.

> - in ":help screenrow()" I noticed a repeated word in:
> "screen row of *the the* cursor

Thanks.

> I attach a patch against Vim-7.3.659 which:
>
> - addresses all above minor issues
> - combine my original patch with your tests in one single
> patch for convenience.

Thanks. One minor issue though, the additional documentation for
screen_screencol()/_screenrow() is not correct, because it starts
from 0 (because I wanted to "export" the static variable
unchanged). The functions in eval.c start from 1.

I updated the comment in the attached patch.

> PS: worth noticing that instead of...
>
> nnoremap <expr> GG ":echom ".screencol()."\n"
>
> ... we can also use:
>
> nnoremap <silent> GG :echom screencol()<CR>

Thanks, I added this as another example to the documentation.

Revised patch (this time definitely from hg tip) attached.

Regards,
Simon

0001-Add-screencol-and-screenrow-add-tests-for-conceal.patch

Simon Ruderich

unread,

Sep 17, 2012, 6:34:09 AM9/17/12

to vim...@googlegroups.com, Bram Moolenaar

Hello,

Is there anything missing to get this patch (with its testcase)
included into Vim?

Reply all

Reply to author

Forward