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.

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

Update vim section movement mappings to support hanging indents. #11492

Closed
wants to merge 1 commit into from

Conversation

wting
Copy link
Contributor

@wting wting commented Jan 12, 2014

By default vim's section motion commands support K&R style braces. This updates the mapping to support Rust's idiomatic style of using hanging indents.

else
nmap <buffer> ][ /\}<CR>b99]}
nmap <buffer> ]] j0[[%/\{<CR>
endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this mapcheck('/') == '' check? That makes it very fragile. I presume you are assuming that the alternative is map / /\v; this is very dangerous: it may well not be the case. This sort of thing is why you should use noremap rather than map. (That means doing things like expanding the [[ in the ]] mapping manually—very well, that's what needs to be done to make it robust.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@chris-morgan
Copy link
Member

There, I'm done with comments for now. If I haven't made myself clear or if you disagree on any points, just holler. :-)

@wting
Copy link
Contributor Author

wting commented Jan 15, 2014

  • remove [] and ][ mappings
  • use noremap for [[
  • add unmappings to undo_plugin

@wting
Copy link
Contributor Author

wting commented Jan 15, 2014

  • use noremap for all mappings

@chris-morgan
Copy link
Member

Thanks, that's fixed up those problems—but after trying it I still find myself unhappy, partly because I didn't think through all the implications (and didn't try it before):

  1. In suggesting map rather than nmap (noremap rather than nnoremap) I had been expecting normal, visual and operator-pending modes to all work; I hadn't thought this through: because of it chaining together a couple of things it actually won't be correct for operator pending mode (d[[ becomes equivalent to d?{<CR>w99[{, which is certainly not what was intended). I'm not certain of the best solution here, but it's bound up in the next matter:
  2. Because it's using ? it hijacks the search register ("/) which is rather nasty behaviour.
  3. When I press [[, the last position (``) should be set to the present cursor location. This does not happen.
  4. It ends up quite a slow operation, especially if 'hlsearch' is on.

I'll see if I can figure a way to fix up these issues.

@chris-morgan
Copy link
Member

I believe I've got it pretty much sorted out now.

diff --git a/src/etc/vim/ftplugin/rust.vim b/src/etc/vim/ftplugin/rust.vim
index 281a63e..dcd4f44 100644
--- a/src/etc/vim/ftplugin/rust.vim
+++ b/src/etc/vim/ftplugin/rust.vim
@@ -42,4 +42,55 @@ if exists("g:loaded_delimitMate")
    let b:delimitMate_excluded_regions = delimitMate#Get("excluded_regions") . ',rustLifetimeCandidate,rustGenericLifetimeCandidate'
 endif

-let b:undo_ftplugin = "setlocal formatoptions< comments< commentstring< includeexpr< suffixesadd< | if exists('b:rust_original_delimitMate_excluded_regions') | let b:delimitMate_excluded_regions = b:rust_original_delimitMate_excluded_regions | unlet b:rust_original_delimitMate_excluded_regions | elseif exists('b:delimitMate_excluded_regions') | unlet b:delimitMate_excluded_regions | endif"
+" Bind motion commands to support hanging indents
+nnoremap <silent> <buffer> [[ :call <SID>Rust_Jump('n', 'Back')<CR>
+nnoremap <silent> <buffer> ]] :call <SID>Rust_Jump('n', 'Forward')<CR>
+xnoremap <silent> <buffer> [[ :call <SID>Rust_Jump('v', 'Back')<CR>
+xnoremap <silent> <buffer> ]] :call <SID>Rust_Jump('v', 'Forward')<CR>
+onoremap <silent> <buffer> [[ :call <SID>Rust_Jump('o', 'Back')<CR>
+onoremap <silent> <buffer> ]] :call <SID>Rust_Jump('o', 'Forward')<CR>
+
+let b:undo_ftplugin = "
+       \setlocal formatoptions< comments< commentstring< includeexpr< suffixesadd<
+       \|if exists('b:rust_original_delimitMate_excluded_regions')
+         \|let b:delimitMate_excluded_regions = b:rust_original_delimitMate_excluded_regions
+         \|unlet b:rust_original_delimitMate_excluded_regions
+       \|elseif exists('b:delimitMate_excluded_regions')
+         \|unlet b:delimitMate_excluded_regions
+       \|endif
+       \|nunmap <buffer> [[
+       \|nunmap <buffer> ]]
+       \|xunmap <buffer> [[
+       \|xunmap <buffer> ]]
+       \|ounmap <buffer> [[
+       \|ounmap <buffer> ]]
+       \"
+
+if exists('*<SID>Rust_Jump') | finish | endif
+
+function! <SID>Rust_Jump(mode, function) range
+   let cnt = v:count1
+   normal! m'
+   if a:mode ==# 'v'
+       norm! gv
+   endif
+   let foldenable = &foldenable
+   set nofoldenable
+   while cnt > 0
+       execute "call <SID>Rust_Jump_" . a:function . "()"
+       let cnt = cnt - 1
+   endwhile
+   let &foldenable = foldenable
+endfunction
+
+function! <SID>Rust_Jump_Back()
+   call search('{', 'b')
+   keepjumps normal! w99[{
+endfunction
+
+function! <SID>Rust_Jump_Forward()
+   normal! j0
+   call search('{', 'b')
+   keepjumps normal! w99[{%
+   call search('{')
+endfunction

There are still a few caveats:

  • Operator-pending mode behaves differently to the standard behaviour: if inside curly braces, it should delete up to and including the closing of the outermost curly brace (that doesn't seem to me consistent with documented behaviour, but it's what it does). Actual behaviour (the more logical and consistent, in my opinion): up to the start of the next outermost curly brace.
  • With folding enabled (set fdm=syntax), [[ and ]] do not behave as they should: the default behaviour treats an entire closed fold as one line for these purposes while this code does not (I explicitly set nofoldenable in the function—the side-effects are worse with folds enabled), leading to unexpected behaviour, the worst of which is [[ and/or ]] not working in visual mode on a closed fold (visual mode keeps it at the extreme end of the region line of the folded region, so it's always going back to the opening line of that fold and immediately being shoved back to the end by visual mode).
  • [[ and ]] are operating inside comments, whereas the standard behaviour skips comments.

@wting
Copy link
Contributor Author

wting commented Jan 18, 2014

Awesome work @chris-morgan! I was content with a minor fix of the [[ and ]] mappings provided in vimdocs, but you've done a lot more work.

Anyway I'm going to close this PR and let you open up your own since at this point none of my suggested changes are relevant. :)

@wting wting closed this Jan 18, 2014
chris-morgan added a commit to chris-morgan/rust that referenced this pull request Feb 27, 2014
(Expressed another way: make `[[` et al. work with the curly brace at
the end of a line as is standard Rust style, not just at the start is it
is by default in Vim, from K&R style.)

This came out of rust-lang#11492, where a simpler but less effective technique
was initially proposed; some discussion of the techniques, ways and
means can be found there.

There are still a few caveats:

- Operator-pending mode behaves differently to the standard behaviour:
  if inside curly braces, it should delete up to and including the
  closing of the outermost curly brace (that doesn't seem to me
  consistent with documented behaviour, but it's what it does). Actual
  behaviour (the more logical and consistent, in my opinion): up to the
  start of the next outermost curly brace.

- With folding enabled (`set fdm=syntax`), `[[` and `]]` do not behave
  as they should: the default behaviour treats an entire closed fold as
  one line for these purposes while this code does not (I explicitly
  `set nofoldenable` in the function—the side-effects are worse with
  folds enabled), leading to unexpected behaviour, the worst of which is
  `[[` and/or `]]` not working in visual mode on a closed fold (visual
  mode keeps it at the extreme end of the region line of the folded
  region, so it's always going back to the opening line of that fold and
  immediately being shoved back to the end by visual mode).

- `[[` and `]]` are operating inside comments, whereas the standard
  behaviour skips comments.

- The viewport position is sometimes changed when it should not be
  necessary.
alexcrichton pushed a commit to alexcrichton/rust that referenced this pull request Feb 28, 2014
(Expressed another way: make `[[` et al. work with the curly brace at
the end of a line as is standard Rust style, not just at the start is it
is by default in Vim, from K&R style.)

This came out of rust-lang#11492, where a simpler but less effective technique
was initially proposed; some discussion of the techniques, ways and
means can be found there.

There are still a few caveats:

- Operator-pending mode behaves differently to the standard behaviour:
  if inside curly braces, it should delete up to and including the
  closing of the outermost curly brace (that doesn't seem to me
  consistent with documented behaviour, but it's what it does). Actual
  behaviour (the more logical and consistent, in my opinion): up to the
  start of the next outermost curly brace.

- With folding enabled (`set fdm=syntax`), `[[` and `]]` do not behave
  as they should: the default behaviour treats an entire closed fold as
  one line for these purposes while this code does not (I explicitly
  `set nofoldenable` in the function—the side-effects are worse with
  folds enabled), leading to unexpected behaviour, the worst of which is
  `[[` and/or `]]` not working in visual mode on a closed fold (visual
  mode keeps it at the extreme end of the region line of the folded
  region, so it's always going back to the opening line of that fold and
  immediately being shoved back to the end by visual mode).

- `[[` and `]]` are operating inside comments, whereas the standard
  behaviour skips comments.

- The viewport position is sometimes changed when it should not be
  necessary.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Sep 25, 2023
…ure, r=Centri3

Fix mutaby used async function argument in closure for `needless_pass_by_ref_mut`

Fixes rust-lang/rust-clippy#11380.

The problem was that it needed to go through closures as well in async functions to correctly find the mutable usage of async function arguments.

changelog: Correctly handle mutable usage of async function arguments in closures.

r? `@Centri3`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants