Fix GetClojureIndent() undesireable cursor move.

[sethyuan]

No cursor move should be observable calling GetClojureIndent(), it breaks some other codes.

[guns]

Hi sethyuan,

I sent a patch for this a problem that sounds just like this, and was added as 7.3.812. Are you using a Vim older than this?

[sethyuan]

Actually I'm using Vim 7.4.x

On 2014Äê5ÔÂ8ÈÕ, at 2:17, Sung Pae notifications@github.com wrote:

Hi sethyuan,

I sent a patch for this a problem that sounds just like this, and was added as 7.3.812. Are you using a Vim older than this?

¡ª Reply to this email directly or view it on GitHub.

[guns]

Could you please provide me an example of how moving the cursor during the indentexpr breaks functionality? This is allowed in the docs for indentexpr:

    The evaluation of the expression must not have side effects!  It must
    not change the text, jump to another window, etc.  Afterwards the
    cursor position is always restored, thus the cursor may be moved.

I am happy to work around any bugs in Vim, but it would also be nice to fix the bugs at the source.

Thank you!

[sethyuan]

Yes, for example:

(let [a b]
  (println a)
  (println b))

And you do cc on the line (println a), the cursor is left on the line (let [a b].

In fact, if you just call GetClojureIndent() on the line (println a), it moves the cursor after returning the result.

[guns]

(let [a b]
  (println a)
  (println b))

And you do cc on the line (println a), the cursor is left on the line (let [a b].

Unfortunately, I can not reproduce this. Could you please:

• Share your vim --version
• Share your platform (OS, console or graphical)?
• Reproduce the bug in a minimal environment:

# In the vim-clojure-static repo:
$ vim -N -u NONE -U NONE -c 'set rtp^=. | filetype plugin indent on | syntax on | setf clojure'

Then run your cc test case. This works as intended on Linux, console vim, 7.4.274.

In fact, if you just call GetClojureIndent() on the line (println a), it moves the cursor after returning the result.

Yes, like the documentation states, Vim restores your cursor position after it calls the indentexpr. Therefore the cursor position after GetClojureIndent() is not supposed to matter. There has been a bug related to indentexpr and cursor position before (discussed in the thread I linked above), so I would not be surprised to learn that there is another.

Thank you for diving deeper into the matter. I very much appreciate it!

[sethyuan]

Hi @guns,

I did some more tests. I disabled all other plugins and ran the cc test again, this time it behaves right. However, when I turned on the paredit plugin the problem arises again. Within the paredit plugin, it uses the &indentexpr to obtain 'GetClojureIndent()' and calls it to obtain the indented number of characters.

[guns]

Well, while current behavior adheres to the indentexpr contract, I can see how GetClojureIndent() makes for a useful library function. So okay, we will change it for the sake of the paredit plugin and possibly others.

Your patch needs a little amendment however: there are three exit points in GetClojureIndent, but your changes do not handle all of them. Could you fix that, then squash your branch?

If you are feeling especially helpful, adding a test case for GetClojureIndent() in clj/test/vim_clojure_static/indent_test.clj would be pretty awesome. I am happy to add it myself if you don't have the time.

Thanks!

[sethyuan]

I've updated the code, please review. Also, please help me add the test, thanks! :)

[guns]

Okay, I merged your patch and added a test. The test is a bit of a hack; we mutate the file after calling GetClojureIndent() and compare the resulting file against an expected version.

The test did catch a bug with your patch, however, so I am glad I wrote it. You were probably misled by the use of lnum for line numbers, and the use of col for column numbers. That's the convention used in the Vim source IIRC, which is why it's a bit inconsistent.

Thanks for the contribution!