Bug: filetypes that rely on &sw fail after the patch which allows sw to be 0

[So8res]

Many filetypes use &sw in their indent files. Most prominent among these is the vim indent file itself.

If you use the new feature that allows sw to be 0 (causing it to fall back to tabstop) then indentation in all of these filetypes breaks.

According to a simple grep, the following filetypes are affected:

ada
awk
bst
cdl
cmake
cobol
css
cucumber
dtd
dylan
eiffel
erlang
eruby
eterm
falcon
fortran
framescript
gitconfig
gitolite
haml
hamster
html
idlang
ishd
java
ld
liquid
logtalk
make
matlab
mma
mp
ocaml
perl
perl6
php
postscr
pov
prolog
python
r
readline
rpl
ruby
sass
sdl
sml
sqlanywhere
tcl
tcsh
tex
tf
tilde
treetop
vb
verilog
vhdl
vim
xf86conf
xinetd
xml
zimbu

I haven't updated my usr/local/share/vim/vim73 files since adding in this patch, are there updated vimfiles that I should be using? If not, is there a plan to update these indent files?

If not, is there a way to make the change less breaking? It seems like there should be a "indent_level()" function of some sort for indenters to use now that &sw can no longer be trusted.

[Bram Moolenaar]

So8res wrote:

> Many filetypes use &sw in their indent files. Most prominent among
> these is the vim indent file itself.
>
> If you use the new feature that allows sw to be 0 (causing it to fall
> back to tabstop) then indentation in all of these filetypes breaks.
>
> According to a simple grep, the following filetypes are affected:
>
> ada

[...]

> zimbu
>
> I haven't updated my usr/local/share/vim/vim73 files since adding in
> this patch, are there updated vimfiles that I should be using? If not,
> is there a plan to update these indent files?
>
> If not, is there a way to make the change less breaking? It seems like
> there should be a "indent_level()" function of some sort for indenters
> to use now that &sw can no longer be trusted.

I don't see a way to avoid using a zero shiftwidth breaking scripts that
use the value of shiftwidth.

We could make &sw return the effective value, but that smells like a
hack. And it would require a way to get the real value, for where the
value is saved and restored.

Using a function to get the effective value of shiftwidth seems like the
best long term solution. It does require changing all the scripts that
now use &sw.

--
No man may purchase alcohol without written consent from his wife.
[real standing law in Pennsylvania, United States of America]

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

[Andy Wokula]

Am 04.10.2012 23:14, schrieb Bram Moolenaar:
>
> So8res wrote:
>
>> Many filetypes use &sw in their indent files. Most prominent among
>> these is the vim indent file itself.
>>
>> If you use the new feature that allows sw to be 0 (causing it to fall
>> back to tabstop) then indentation in all of these filetypes breaks.
>>
>> According to a simple grep, the following filetypes are affected:
>>
>> ada
>
> [...]
>
>> zimbu
>>
>> I haven't updated my usr/local/share/vim/vim73 files since adding in
>> this patch, are there updated vimfiles that I should be using? If not,
>> is there a plan to update these indent files?
>>
>> If not, is there a way to make the change less breaking? It seems like
>> there should be a "indent_level()" function of some sort for indenters
>> to use now that &sw can no longer be trusted.
>
> I don't see a way to avoid using a zero shiftwidth breaking scripts that
> use the value of shiftwidth.
>
> We could make &sw return the effective value, but that smells like a
> hack. And it would require a way to get the real value, for where the
> value is saved and restored.
>
> Using a function to get the effective value of shiftwidth seems like the
> best long term solution. It does require changing all the scripts that
> now use &sw.

Name suggestions:
shiftwidth()
indentunit()

--
Andy

[Jokcy Lou]

I am sorry for disturbing you. I am new here to join the develop team
of vim. But actually I don't know what to do now. May be someone will
give me a message about how to start. Thank you

2012/10/5 Andy Wokula <anw...@yahoo.de>:

> --
> You received this message from the "vim_dev" maillist.
> Do not top-post! Type your reply below the text you are replying to.
> For more information, visit http://www.vim.org/maillist.php

[So8res]

1. Add a function that returns the indent level that the user desires. (Basically exposes get_sw_value() to the user.)*

2. Modify the existing ftplugins to use indent() instead of &shiftwidth. Should be pretty easy with awk, s/(&(?:sw|shiftwidth))/indent()/.

3. Post a patch.

* We can either modify the indent() function to return the shift unit when given no params (patch below) or create a new function (shiftwidth()?). Thoughts?

Attached is a patch that implements the first option. I've done cursory tests but obviously not conclusive ones for all indent file changes. Should be pretty easy to implement a different option, for those who care a quick perl script to fix things up is:

» find runtime/ -type f | xargs perl -pi -e 's/(&(sw|shiftwidth))/<FUNCTION_NAME>()/g'

(Is there a way to access the value of &sw without using the & sign? If not then this should be a pretty surefire way to fix things.)

On Friday, October 5, 2012 3:31:07 AM UTC-7, Jokcy Lou wrote:
> I am sorry for disturbing you. I am new here to join the develop team
>
> of vim. But actually I don't know what to do now. May be someone will
>
> give me a message about how to start. Thank you
>
>
>

> 2012/10/5 Andy Wokula <>:

[Ben Fritz]

On Friday, October 5, 2012 10:36:20 AM UTC-5, So8res wrote:
> 1. Add a function that returns the indent level that the user desires. (Basically exposes get_sw_value() to the user.)*
>
>
>
> 2. Modify the existing ftplugins to use indent() instead of &shiftwidth. Should be pretty easy with awk, s/(&(?:sw|shiftwidth))/indent()/.
>
>
>
> 3. Post a patch.
>
>
>
> * We can either modify the indent() function to return the shift unit when given no params (patch below) or create a new function (shiftwidth()?). Thoughts?
>
>
>
> Attached is a patch that implements the first option. I've done cursory tests but obviously not conclusive ones for all indent file changes. Should be pretty easy to implement a different option, for those who care a quick perl script to fix things up is:
>
>
>
> » find runtime/ -type f | xargs perl -pi -e 's/(&(sw|shiftwidth))/<FUNCTION_NAME>()/g'
>
>
>
> (Is there a way to access the value of &sw without using the & sign? If not then this should be a pretty surefire way to fix things.)
>

getbufvar, getwinvar, gettabwinvar functions can be used, and those will use the & sign, but replacing it in this case would cause the script to break.

Also some scripts might use &sw to SET the value (unlikely in the official runtime but possible).

So you'll need to review your changes before submitting them.

[Ben Fritz]

For example, this hunk is wrong from eval.txt in your patch:

@@ -3754,9 +3756,9 @@
Like |input()|, but when the GUI is running and text dialogs
are supported, a dialog window pops up to input the text.
Example: >
- :let n = inputdialog("value for shiftwidth", &sw)
+ :let n = inputdialog("value for shiftwidth", indent())
:if n != ""
- : let &sw = n
+ : let indent() = n
:endif
< When the dialog is cancelled {cancelreturn} is returned. When
omitted an empty string is returned.

[Ben Fritz]

On Friday, October 5, 2012 10:36:20 AM UTC-5, So8res wrote:

> 1. Add a function that returns the indent level that the user desires. (Basically exposes get_sw_value() to the user.)*
>
>
>
> 2. Modify the existing ftplugins to use indent() instead of &shiftwidth. Should be pretty easy with awk, s/(&(?:sw|shiftwidth))/indent()/.
>
>
>
> 3. Post a patch.
>
>
>
> * We can either modify the indent() function to return the shift unit when given no params (patch below) or create a new function (shiftwidth()?). Thoughts?
>
>
>
> Attached is a patch that implements the first option. I've done cursory tests but obviously not conclusive ones for all indent file changes. Should be pretty easy to implement a different option, for those who care a quick perl script to fix things up is:
>
>
>

Your patch fixes both the runtime files and the C code. While both are needed, Bram (almost?) always pushes runtime file updates separately from code changes. So teasing it apart into two patches is probably a better idea, especially because currently each runtime file has a different maintainer.

[Bram Moolenaar]

So8res wrote:

> 1. Add a function that returns the indent level that the user desires.
> (Basically exposes get_sw_value() to the user.)*
>
> 2. Modify the existing ftplugins to use indent() instead of
> &shiftwidth. Should be pretty easy with awk,
> s/(&(?:sw|shiftwidth))/indent()/.
>
> 3. Post a patch.
>
> * We can either modify the indent() function to return the shift unit
> when given no params (patch below) or create a new function
> (shiftwidth()?). Thoughts?

I prefere a new function. indent() returning two different things is
slightly confusing.

> Attached is a patch that implements the first option. I've done
> cursory tests but obviously not conclusive ones for all indent file
> changes. Should be pretty easy to implement a different option, for
> those who care a quick perl script to fix things up is:
>
> » find runtime/ -type f | xargs perl -pi -e 's/(&(sw|shiftwidth))/<FUNCTION_NAME>()/g'

The problem is not so much changing &sw to shiftwidth(), but getting all
maintainers to do this.


--
FROG: How you English say: I one more time, mac, I unclog my nose towards
you, sons of a window-dresser, so, you think you could out-clever us
French fellows with your silly knees-bent creeping about advancing
behaviour. (blows a raspberry) I wave my private parts at your aunties,
you brightly-coloured, mealy-templed, cranberry-smelling, electric
donkey-bottom biters.
"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

[So8res]

Right, I missed the cases where people are setting &sw. Below is the updated patch. I've also separated out the runtime/ changes and the indent() changes.

Are people happy with re-using indent() here or should we add a shiftwidth() function?

Maintainers are going to be the bottleneck here. What's usual policy for backwards incompatible changes? This is an interesting case in that it's an opt-in backwards-incompatible change, so it won't break anything unexpectedly. But how do we announce to people that they need to update, and can we update a chunk of the runtime files ourselves?

[So8res]

Oops, missed your message, Bram. I'll whip up a new function next week, I'm busy travelling.

[So8res]

[James McCoy]

On Sat, Oct 06, 2012 at 10:38:10AM -0700, So8res wrote:
> diff -r 0769b84adf93 runtime/doc/eval.txt
> --- a/runtime/doc/eval.txt Fri Oct 05 21:30:08 2012 +0200
> +++ b/runtime/doc/eval.txt Sat Oct 06 10:21:36 2012 -0700
> @@ -3754,7 +3754,7 @@

> Like |input()|, but when the GUI is running and text dialogs
> are supported, a dialog window pops up to input the text.
> Example: >
> - :let n = inputdialog("value for shiftwidth", &sw)

> + :let n = inputdialog("value for shiftwidth", &sw())

This should be "shiftwidth()", not "&sw()".

--
James
GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy <jame...@jamessan.com>

[So8res]

Fixed. What's the process for moving forward with this patch?

[Bram Moolenaar]

James wrote:

> Fixed. What's the process for moving forward with this patch?

Thanks. I'll check it out soon and probably include it.

Problem is that when we update indent files to use shiftwidth() they
won't work with older versions of Vim. Can you think of a simple way to
use shiftwidth() only when available? Possibly by defining a
s:Shiftwidth() function that uses has() and falls back to &sw.


--
CART DRIVER: Bring out your dead!
We follow the cart through a wretched, impoverished plague-ridden village.
A few starved mongrels run about in the mud scavenging. In the open
doorway of one house perhaps we jug glimpse a pair of legs dangling from
the ceiling. In another doorway an OLD WOMAN is beating a cat against a
wall rather like one does with a mat. The cart passes round a dead donkey
or cow in the mud. And a MAN tied to a cart is being hammered to death by
four NUNS with huge mallets.

[So8res]

Dang, I was hoping the average person wouldn't get the updated vimfiles unless they downloaded the cutting edge vim.

The easy solution is to use (&sw?&sw:&ts) in place of shiftwidth() and just recommend that people use shiftwidth() going forward. But if that's too ugly then yeah I can go through and add s:Shiftwidth() to things.

[ZyX]

> Thanks. I'll check it out soon and probably include it.
>
> Problem is that when we update indent files to use shiftwidth() they
> won't work with older versions of Vim. Can you think of a simple way to
> use shiftwidth() only when available? Possibly by defining a
> s:Shiftwidth() function that uses has() and falls back to &sw.

I would rather suggest not having s:Shiftwidth with has(), but have has() at the top level:

if exists('*shiftwidth')
let s:Shiftwidth=function('shiftwidth')
else
function s:Shiftwidth()
return &sw
endfunction
endif

[So8res]

Good idea.

[So8res]

Aaand two problems with the above patches:
1. Forgot to declare f_shiftwidth as static void
2. syntax error declaring s:Shiftwidth() (yes I'm embarrassed, I tested against a build that didn't allow sw=0...)

Here are the fixed ones: