Skip to content

Have ci[ look for next [ like ci" does for next " #8670

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

Closed
wants to merge 3 commits into from
Closed

Have ci[ look for next [ like ci" does for next " #8670

wants to merge 3 commits into from

Conversation

cls
Copy link
Contributor

@cls cls commented Jul 30, 2021

In runtime/doc/todo.txt is a known issue:

"ci[" does not look for next [ like ci" does look for next ".
(J.F. 2017 Jan 7)

For instance, if you are at the beginning of a line that reads #include "foo.h" then ci" will change the contents of the string, but if it reads #include <foo.h> then ci< will fail to find and change the contents of the angle brackets.

This patch has ci[ search forward for a bracket like ci" does.

There are two commits. The first is sufficient for a plain i[ or 1i[, but behaves a little strangely with numbers greater than 1. Specifically, di[ before a[b[c]d]e leaves a[]e, and 2di[ leaves a[b[]d]e, but any odd does the same as 1, and any even 2. This is just an artefact of the way it searches for a match. i" doesn't have this problem as 2+ is just a single special case.

The second commit fixes this so that 3i[ for example will enter the third nested bracket it finds, in a sort of inside-out version of what it would do if you were inside the brackets rather than outside of them. A higher number than there are nested brackets will fail. It's not entirely clear that this is the ideal behaviour, and it's a bit more code which is why I've kept it separate, but it seemed fairly reasonable.

This patch does not handle tag-blocks (it), only 'regular' blocks of '(', '{', etc.

Please let me know if there are any issues. Thanks.

Copy link
Member

@yegappan yegappan left a comment

Choose a reason for hiding this comment

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

Can you add a test for this change?

@cls
Copy link
Contributor Author

cls commented Jul 30, 2021

I've added a test, though I'm not especially familiar with Vimscript so hopefully it's acceptable. Running the test locally, it passes with my changes and fails without.

@vim-ml
Copy link

vim-ml commented Jul 30, 2021 via email

@codecov
Copy link

codecov bot commented Jul 30, 2021

Codecov Report

Merging #8670 (c88c960) into master (c9e7e34) will increase coverage by 0.00%.
The diff coverage is 96.79%.

❗ Current head c88c960 differs from pull request most recent head 556f4b5. Consider uploading reports for the commit 556f4b5 to get more accurate results
Impacted file tree graph

@@           Coverage Diff            @@
##           master    #8670    +/-   ##
========================================
  Coverage   90.06%   90.07%            
========================================
  Files         150      146     -4     
  Lines      169172   169476   +304     
========================================
+ Hits       152372   152651   +279     
- Misses      16800    16825    +25     
Flag Coverage Δ
huge-clang-none 89.37% <95.22%> (+0.27%) ⬆️
huge-gcc-none 89.59% <95.73%> (+0.03%) ⬆️
huge-gcc-testgui 88.22% <96.71%> (+0.04%) ⬆️
huge-gcc-unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/blob.c 92.48% <ø> (-0.18%) ⬇️
src/if_perl.xs 90.92% <ø> (-0.41%) ⬇️
src/if_python.c 83.27% <0.00%> (ø)
src/if_python3.c 87.29% <0.00%> (ø)
src/if_ruby.c 91.60% <ø> (ø)
src/if_tcl.c 90.38% <ø> (ø)
src/memline.c 88.77% <ø> (ø)
src/version.c 92.34% <ø> (ø)
src/crypt.c 84.46% <66.66%> (-1.31%) ⬇️
src/undo.c 84.66% <70.00%> (-0.10%) ⬇️
... and 106 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9e7e34...556f4b5. Read the comment docs.

@gdupras
Copy link
Contributor

gdupras commented Jul 31, 2021

I tried your patch and I've noticed a difference in behavior when comparing di[ with di".

With the cursor on the first line:

1
2
[
3
4
5
]
6

doing di[ gives

1
2
[
]
6

So di[ also searches below and above the current line. However di" only searches the current line. With the cursor on the first line:

1
2
"
3
4
5
"
6

doing di" does nothing.

I don't mind but I thought I would point it out, especially since you don't have tests with multiple lines.

@brammool
Copy link
Contributor

di" and di[ work differently, because the double quote does not indicate a direction. Therefore it only works in one line, assuming that the first double quote in the line is the start of the object. For a square bracket we can start anywhere, and know that [ is the start and ] is the end.

@brammool brammool closed this in b9115da Jul 31, 2021
chrisbra pushed a commit to chrisbra/vim that referenced this pull request Aug 30, 2021
Problem:    ci" finds following string but ci< and others don't.
Solution:   When not inside an object find the start. (Connor Lane Smit,
            closes vim#8670)
mliszcz added a commit to mliszcz/neovim that referenced this pull request Nov 15, 2021
Problem:    ci" finds following string but ci< and others don't.
Solution:   When not inside an object find the start. (Connor Lane Smit,
            closes vim/vim#8670)
vim/vim@b9115da
mliszcz added a commit to mliszcz/neovim that referenced this pull request Nov 15, 2021
Problem:    ci" finds following string but ci< and others don't.
Solution:   When not inside an object find the start. (Connor Lane Smit,
            closes vim/vim#8670)
vim/vim@b9115da
mliszcz added a commit to mliszcz/neovim that referenced this pull request Nov 18, 2021
Problem:    ci" finds following string but ci< and others don't.
Solution:   When not inside an object find the start. (Connor Lane Smit,
            closes vim/vim#8670)
vim/vim@b9115da
janlazo pushed a commit to neovim/neovim that referenced this pull request Nov 21, 2021
…'t (#16324)

Problem:    ci" finds following string but ci< and others don't.
Solution:   When not inside an object find the start. (Connor Lane Smit,
            closes vim/vim#8670)
vim/vim@b9115da
Shougo pushed a commit to Shougo/neovim that referenced this pull request Dec 1, 2021
…'t (neovim#16324)

Problem:    ci" finds following string but ci< and others don't.
Solution:   When not inside an object find the start. (Connor Lane Smit,
            closes vim/vim#8670)
vim/vim@b9115da
AlexPl292 added a commit to JetBrains/ideavim that referenced this pull request Jul 14, 2022
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.

5 participants