8000 Use shiftwidth() instead of &sw in indent/vim.vim by tyru · Pull Request #578 · vim/vim · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use shiftwidth() instead of &sw in indent/vim.vim #578

Closed
wants to merge 3 commits into from
Closed

Use shiftwidth() instead of &sw in indent/vim.vim #578

wants to merge 3 commits into from

Conversation

tyru
Copy link
@tyru tyru commented Jan 23, 2016

With set shiftwidth=0, gg=G in filetype=vim buffer will result in removing all indents.
Because indent/vim.vim doesn't use shiftwidth() instead of accessing &sw directly.

@tyru tyru mentioned this pull request Jan 23, 2016
Closed
@brammool
Copy link
Contributor

Tyru wrote:

With set shiftwidth=0, gg=G in filetype=vim buffer will result
in removing all indents.
Because indent/vim.vim doesn't use shiftwidth() instead of
accessing &sw directly.

I'll include it, thanks!

My sister Cecilia opened a computer store in Hawaii.
She sells C shells by the seashore.

/// Bram Moolenaar -- Bram@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 ///

Sorry, something went wrong.

@mattn
Copy link
Member
mattn commented Jan 24, 2016

there are many using of &sw.
https://gist.github.com/mattn/f2d00a2b26044d840a4c

Those should be fixed.

@tyru
Fix all &sw references in indent plugins
29d92a9 
Use `shiftwidth()` instead.
@crazymaster crazymaster mentioned this pull request Jan 24, 2016
Closed
@tyru
Copy link
Author
tyru commented Jan 24, 2016

I substituted all 'shiftwidth' references in all indent plugins. (Thanks @mattn)
And there were some thoughts while seeing indent plugins.

Using shiftwidth() function wrapper

I think shiftwidth() function wrapper is not necessary.
Because runtime files should not be distributed with an older Vim.

  • bzl.vim
  • go.vim
  • html.vim
  • teraterm.vim
  • yaml.vim

Already using shiftwidth() function

  • j.vim
  • python.vim

Not simply substituted

  • ocaml.vim
    • Before: &sw + &sw
    • After: shiftwidth() * 2
  • pascal.vim
    • Before: return indnt - &shiftwidth - &shiftwidth
    • After: return indnt - (shiftwidth() * 2)
  • sml.vim
    • Before: &sw + &sw
    • After: shiftwidth() * 2

Other

  • Simply substitute like :%s/&sw\|&shiftwidth/shiftwidth()/g
    • By hand, though...

Misc.

  • runtime/indent/lifeline.vim has an executable bit?
    • -rwxr-xr-x on my system
  • Some indent plugin changes 'shiftwidth',
    but if it is already 0, it should not be changed?
    • Because if 'shiftwidth' is 0,
      user may want to sync the value with 'tabstop'.
    • Here is the list of plugins which change 'shiftwidth' value.
      • clojure.vim
        • setlocal softtabstop=2 shiftwidth=2 expandtab
      • haml.vim
        • setlocal autoindent sw=2 et
      • sass.vim
        • setlocal autoindent sw=2 et
      • sml.vim
        • setlocal shiftwidth=2
      • tcsh.vim
        • setlocal indentkeys+=e,0=end,0=endsw indentkeys-=0{,0},0),:,0#
      • zimbu.vim
        • setlocal sw=2 et

@tyru
Copy link
Author
tyru commented Jan 24, 2016

I think shiftwidth() function wrapper is not necessary.
Because runtime files should not be distributed with an older Vim.

Or, @mattn said here in japanese.

Personally, I think we should create a dedicated autoload function and use it in all indent plugins.

@brammool
Copy link
Contributor

Tyro wrote:

I substituted all 'shiftwidth' references in all indent plugins.
(Thanks @mattn)
And there were some thoughts while seeing indent plugins.

Using shiftwidth() function wrapper

I think shiftwidth() function wrapper is not necessary.
Because runtime files should not be distributed with an older Vim.

  • bzl.vim
  • go.vim
  • html.vim
  • teraterm.vim
  • yaml.vim

Already using shiftwidth() function

  • j.vim
  • python.vim

Not simply substituted

  • ocaml.vim
    • Before: &sw + &sw
    • After: shiftwidth() * 2
  • pascal.vim
    • Before: return indnt - &shiftwidth - &shiftwidth
    • After: return indnt - (shiftwidth() * 2)
  • sml.vim
    • Before: &sw + &sw
    • After: shiftwidth() * 2

Other

  • Simply substitute like :%s/&sw\|&shiftwidth/shiftwidth()/g
    • By hand, though...

Misc.

  • runtime/indent/lifeline.vim has an executable bit?
    • -rwxr-xr-x on my system
  • Some indent plugin changes 'shiftwidth',
    but if it is already 0, it should not be changed?
    • Because if 'shiftwidth' is 0,
      user may want to sync the value with 'tabstop'.
    • Here is the list of plugins which change 'shiftwidth' value.
      • clojure.vim
        • setlocal softtabstop=2 shiftwidth=2 expandtab
      • haml.vim
        • setlocal autoindent sw=2 et
      • sass.vim
        • setlocal autoindent sw=2 et
      • sml.vim
        • setlocal shiftwidth=2
      • tcsh.vim
        • setlocal indentkeys+=e,0=end,0=endsw indentkeys-=0{,0},0),:,0#
      • zimbu.vim
        • setlocal sw=2 et

By now it should be OK to use shiftwidth() directly. It was added in
patch 7.4.694, more than three years ago.

Please send patches to each maintainer.

Those who live by the sword get shot by those who don't.

/// Bram Moolenaar -- Bram@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 ///

@k-takata
Copy link
Member

Please send patches to each maintainer.

Oh, what a hard work...
Maybe, we need an announcement to the maintainers.
And..., you should merge the patch to the zimbu.vim.

@mattn mattn mentioned this pull request Jan 25, 2016
Closed
@tyru
Copy link
Author
tyru commented Jan 25, 2016

@brammool Okay :)
I should generate maintainer's list according to https://gist.github.com/tyru/5fc70f0ce5be91712838

  1. Use shiftwidth() directly.
  2. Generate address list according to https://gist.github.com/tyru/5fc70f0ce5be91712838
    • More investigations about (not found) files.
  3. Send patches to maintainers.

@tyru
Copy link
Aut 8000 hor
tyru commented Jan 25, 2016

Some indent plugin changes 'shiftwidth',
but if it is already 0, it should not be changed?

@brammool
How do you think about this?
I think all indent plugins should not change shiftwidth and softtabstop.
At least when those values are 0(shiftwidth) or negative(softtabstop).

@tyru
Copy link
Author
tyru commented Jan 25, 2016

@brammool
If I should contact with each maintainer and ask them about shiftwidth and softtabstop, I will do that :)
I suppose you have many tasks other than this issue...
However, thank you for including indent/vim.vim and developing Vim!

@tyru
Remove a checking code / a wrapper function for shiftwidth()
44f4d91 
Removed a checking code / a wrapper function for shiftwidth().
* bzl.vim
* go.vim
* html.vim
* teraterm.vim
* yaml.vim
@tyru
Copy link
Author
tyru commented Jan 25, 2016

Sent.
https://groups.google.com/forum/#!topic/vim_dev/R046Odhv8rY

@brammool
Copy link
Contributor

Tyru wrote:

Some indent plugin changes 'shiftwidth',
but if it is already 0, it should not be changed?

@brammool
How do you think about this?
I think all indent plugins should not change shiftwidth and softtabstop.
At least when those values are 0(shiftwidth) or negative(softtabstop).

Normally the user has his preference for 'shiftwidth' and we should
respect that. However, for some file formats there is a recommended or
even enforced indent policy. E.g. for Makefile one should use a Tab in
front of a command.

Microsoft is to software what McDonalds is to gourmet cooking

/// Bram Moolenaar -- Bram@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 ///

@tyru
Copy link
Author
tyru commented Jan 26, 2016

However, for some file formats there is a recommended or even enforced indent policy.

I agree.
I want to know the reason why some plugins set shiftwidth or softtabstop.
If it is NOT a enforced indent policy for the filetype, I don't think it should change shiftwidth or softtabstop.

And though this is my humble assumption, all indent plugins listed above don't seem to have a enforced indent policy.
It seems a just recommended setting by who wrote this setting (Maybe maintainer).
Or, If indent policy is strongly recommended by its programming language coding guideline, I think it is valid.

At least, I suppose you know the reason well for zimbu.vim :)

@brammool
Copy link
Contributor

Takuya Fujiwara wrote:

However, for some file formats there is a recommended or even
enforced indent policy.

I agree.
I want to know the reason why some plugins set shiftwidth or
softtabstop.
If it is NOT a enforced indent policy for the filetype, I don't think it should change shiftwidth or softtabstop.

And though this is my humble assumption, all indent plugins listed
above don't seem to have a enforced indent policy.
It seems a just recommended setting by who wrote this setting (Maybe
maintainer).
Or, If indent policy is strongly recommended by its programming
language coding guideline, I think it is valid.

At least, I suppose you know the reason well for zimbu.vim :)

Yes, since Zimbu is a new language we can enforce a certain style.
And it doesn't allow any tabs. Reduces the choices that a programmer
has to make, less time wasted on discussions about coding style.
And makes the code easier to read.

hundred-and-one symptoms of being an internet addict:
45. You buy a Captain Kirk chair with a built-in keyboard and mouse.

/// Bram Moolenaar -- Bram@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 ///

@tyru
Copy link
Author
tyru commented Jan 27, 2016

And it doesn't allow any tabs. Reduces the choices that a programmer
has to make, less time wasted on discussions about coding style.

I see. sounds good :)
It looks to me like zimbu.vim has a solid reason to set shiftwidth.

@vim-ml
Copy link
vim-ml commented Jan 27, 2016

2016-01-26 22:10 GMT+03:00 Bram Moolenaar vim-dev-github@256bit.org:

Takuya Fujiwara wrote:

However, for some file formats there is a recommended or even
enforced indent policy.

I agree.
I want to know the reason why some plugins set shiftwidth or
softtabstop.
If it is NOT a enforced indent policy for the filetype, I don't think it
should change shiftwidth or softtabstop.

And though this is my humble assumption, all indent plugins listed
above don't seem to have a enforced indent policy.
It seems a just recommended setting by who wrote this setting (Maybe
maintainer).
Or, If indent policy is strongly recommended by its programming
language coding guideline, I think it is valid.

At least, I suppose you know the reason well for zimbu.vim :)

Yes, since Zimbu is a new language we can enforce a certain style.
And it doesn't allow any tabs. Reduces the choices that a programmer
has to make, less time wasted on discussions about coding style.
And makes the code easier to read.

​The fact that tabs are banned in the spec justifies only setting
&expandtab.​ But I do not see Zimbu specification talking about exact
amount of spaces indentation, so &shiftwidth or &softtabstop settings
should not be there (or specification should be updated; I did not read it
though, just searched for “space” and “indent”).

hundred-and-one symptoms of being an internet addict:
45. You buy a Captain Kirk chair with a built-in keyboard and mouse.

/// Bram Moolenaar -- Bram@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 ///

—
Reply to this email directly or view it on GitHub
#578 (comment).

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

You received this message because you are subscrib 8000 ed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to vim_dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

@chrisbra
Copy link
Member

can this be closed?

@brammool
Copy link
Contributor

Change was included for vim.vim. Other files still to be done, but that's not what this issue is for.

@brammool brammool closed this Jan 31, 2016
@tyru
Copy link
Author
tyru commented Feb 2, 2016

Thanks!

I created the list of vim indent plugin maintainers to summarize current status for myself.
I don't if it is useful, share it here.
https://docs.google.com/spreadsheets/d/1-HeWlsLC-tJDfuXQjtkZmY5UwXakAuYg83z9nz7nl2w/edit?usp=sharing

chrisbra added a commit to chrisbra/vim-sh-indent that referenced this pull request Feb 2, 2016
@chrisbra
use shiftwidth() function
3a83e83 
fixes: vim/vim#578
@vim-ml
Copy link
vim-ml commented Feb 2, 2016

Hi Takuya!

On Di, 02 Feb 2016, Takuya Fujiwara wrote:

I created the list of vim indent plugin maintainers to summarize current status for myself.
I don't if it is useful, share it here.
https://docs.google.com/spreadsheets/d/1-HeWlsLC-tJDfuXQjtkZmY5UwXakAuYg83z9nz7nl2w/edit?usp=sharing

I'll attach an updated indent script here.

@bram, please include the sh.indent script

Best,

Christian

Argwöhnisch wacht der Mensch über alles, was ihm gehört - nur die Zeit
läßt er sich oft stehlen.
-- Linus Carl Pauling

@vim-ml
Copy link
vim-ml commented Feb 2, 2016

Christian Brabandt wrote:

On Di, 02 Feb 2016, Takuya Fujiwara wrote:

I created the list of vim indent plugin maintainers to summarize current =
status for myself.
I don't if it is useful, share it here.
https://docs.google.com/spreadsheets/d/1-HeWlsLC-tJDfuXQjtkZmY5UwXakAuYg8=
3z9nz7nl2w/edit?usp=3Dsharing

I'll attach an updated indent script here.

@bram, please include the sh.indent script

Thanks. I'll update the revision date.

hundred-and-one symptoms of being an internet addict:
111. You and your friends get together regularly on IRC, even though
all of you live in the same city.

/// Bram Moolenaar -- Bram@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 ///

2072 added a commit to 2072/vim that referenced this pull request Feb 7, 2016
@2072
Update the PHP indent script to version 1.60
c501210 
Changes since 1.57:

- Check shiftwidth() instead of 'shiftwidth' (will use the 'tabstop' value if 'shiftwidth' is 0) (PR 2072/PHP-Indenting-for-VIm#39) (related to vim#578)
- Add support for optional spaces (before only) and double quotes around (Here|Now)Document identifiers (issue 2072/PHP-Indenting-for-VIm#40)
- Multi-line indenting could get wrong whenever started on a commented line. (issue 2072/PHP-Indenting-for-VIm#44)
@2072 2072 mentioned this pull request Feb 7, 2016
Closed
This was referenced Feb 8, 2016
Closed
Closed
Closed
Closed
Merged
@tyru tyru deleted the vim-indent-shiftwidth branch September 2, 2016 13:10
@tyru
Copy link
Author
tyru commented Sep 2, 2016 •
edited
Loading

Thanks Bram for fixing this issue completely in 89bcfda.

@tyru tyru mentioned this pull request Feb 19, 2017
Closed
@tyru tyru mentioned this pull request Mar 7, 2017
Closed
@tyru tyru mentioned this pull request Oct 20, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tyru @brammool @mattn @k-takata @vim-ml @chrisbra
306F
0