Codecov Report

Merging #11976 (d5ed4a5) into master (094b847) will decrease coverage by 0.48%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #11976       /-   ##
==========================================
- Coverage   81.93%   81.45%   -0.48%     
==========================================
  Files         164      164              
  Lines      193978   193858     -120     
  Branches    43811    43756      -55     
==========================================
- Hits       158927   157914    -1013     
- Misses      22211    23150      939     
  Partials    12840    12794      -46     

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

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

You cannot decide what is a string without looking in surrounding lines.
You appear to assume that strings can't cross a line break. That is not
always the case.

But we say that the i{quote} and a{quote} text object only operates on a line, so that is a reasonable assumption

a"							*v_aquote* *aquote*
a'							*v_a'* *a'*
a`							*v_a`* *a`*
			"a quoted string".  Selects the text from the previous
			quote until the next quote.  The 'quoteescape' option
			is used to skip escaped quotes.
			Only works within one line.

The existing behavior extends the Visual area, your changed behavior
appears to keep only the last selected object. I do not see that as an
improvement.

Personally I find selecting quoted strings to be much more helpful than extending the area to the next quoted string. I cannot think of where it would be useful of the top of my head and that's why I changed it like that.

I'm not sure if selecting an object backwards is a good idea. This also
makes it a bit unpredictable.

Say the case is like the following

"string" stuff|

With this patch only in such cases when unambigously when there is no quoted string forwards and there is a quoted string backwards do I chose to select it backwards, otherwise it always chooses forwards like how it does now.

This makes it complicated. The user could move the cursor before
selecting the object. It's often better to have more than one simple
command than one complicated one

That's the current behaviour though. I was just listing out what the quote object should do (including existing and new additions)

Isn't this how it is already working?

No, vi'i' selects the quotes but vi'i'i' selects the quotes and then extends the visual selection (which I think is unhelpful in most cases). With this patch it selects the next string.

Generally a count means repeating the command, thus "2i'" would do the
same as "i'i'". Doing otherwise is unexpected. Is there a really good
reason to do something else in this case?

the ability for i' to respect the count given to it enables doing things like c3i' or d3i' (changing and deleting the 3rd string respectively). repeating i' is only possible in visual mode anyway like vi'i'. Doing ci'i' and di'i'are not supported. Currently v2i' selects the quotes around an object, with this patch v2i', c2i' and d2i' operate on the second string. We can mention that only i'i' is special cased for visual mode (like it already was).

This makes sense. But what if the cursor is in the "comment" text, it
would select something else? Then it isn't different from the existing
behavior, is it?

Yes, the behaviour only differs when there are an even number of quotes in a line.

The user can move the cursor to another
location to make the selection cover the right text

And this patch enables you do to that? I am not sure I understand this comment

And it is useful in a list of
strings

But, here's where this new behaviour of operating on the next string is useful,
and why the trade-off of it not working nicely with list of strings for example is worth it.

Say you're on a long line:

"string" and then a lot of text with lots of words and possibly some punctuation, but here is another "string"

now say you're inside the first string and want to select the second one, you
know that most vim motions accept a count do you v2i" ah but that selects the
quotes, no problem you press i" again and now you have selected the entire
line (not what you wanted) , so you escape and then the cursor is inside the
correct string, you try to do vi" again and it has finally selected the string
you wanted.

So, naturally while editing you go v2i"i"<esc>i" to select the next string. If
someone is acquainted with this behaviour then they might do something along the
lines of f";vi" or just f"vi" depending on where they are. With this patch you
just do v2i" to select the next string and it reads like second double quoted string as well.

This is helpful while not only editing prose but while editing code as well, take the
following contrived example:

parsed = map(lambda line: map(partial(parser, prefix="0b") , line.split()),  with_iter(open("./file.txt")).readlines())

and now say that you're on line.split() you want to change the file.txt to
binary_data.txt well you know that vim will search forwards for a "quoted string"
from past experience and proceed to do ci" and what happens is the following.

parsed = map(lambda line: map(lambda word: "text"|"./file.txt")).readlines())

I hope you will agree that that is not very helpful.
With this patch the following happens when you press ci".

parsed = map(lambda line: map(partial(parser, prefix="0b") , line.split()), with_iter(open("|")).readlines())

It operated the c on the next balanced i" object.
Which is hopefully inline with what most people expect when they tell vim
to operate on the next quoted string using the i" textobject.

Say a person in some case does want to select that region, then they can do
F"ct" which is similar to what you would do in the above example to select the
balanced quote object (f"ci") and its the same number of keystrokes as well.
In that case why not make the more useful case (of selecting balanced quotes)
more friction free by making it part of the semantics of i" and others.

Infact it is documented as "a quoted string" but as I said originally (and shown here),
it doesn't always select a quoted string.

I found the following entry in the todo.txt

I am not sure if they meant this behaviour, but perhaps this new behaviour I am proposing is more logical?
To me it is certainly more intuitive.

The issue that led to the aforementioned todo entry was probably the following mail by Ben Fritz

I noticed two behaviors of i" and a" that surprised me.

Consider this text:

"Here's a patch," Daniel said.

First, set selection=exclusive.

Now place the cursor on the beginning of the line and type va". This selects the whole string "Here's a patch," with the quotes and one trailing whitespace, as expected. Typing va" again should have no effect, because there are no strings to select. Instead Vim beeps and the trailing whitespace is no longer selected.

Repeating the same process with vi" instead of va" unselects the comma at the end of the string. Repeating further times toggles the comma between selected and unselected.

That was issue 1.

Now use text:

"Thanks," Bram said. "I'll put it on the TODO list!"

Again place the cursor at the beginning of the line. va" selects the first string, "Thanks," and trailing whitespace as expected. Type va" again and both strings are now selected, which is documented. So far, so good.

Now try again with vi". This is not supposed to do anything, as the :help says:

  	Like a", a' and a`, but exclude the quotes and
  	repeating won't extend the Visual selection.

Instead, the entire first string gets unselected, and the entire second string "I'll put it on the TODO list!" gets selected instead (with the quotes this time). vi" again unselects the trailing quote but not the leading quote. Once the trailing quote is unselected, issue 1 applies again.

That was issue 2.

This seems to be different from what you try to solve/change.

Apart from that I think you are restricting your view to much on source code, where quoted text spanning multiple lines is quite seldom. But if you edit natural language and write quoted speech, you will soon have quotes spanning multiple lines where it is hard to find the opening quote without understanding the text. That's an inherent disadvantage of having identical opening and closing symbols.

In my opinion Vim's rule to act on the first pair of quotes surrounding the cursor on the current line is a good compromise. Everything else would probably result in unpredictable behavior.

In my opinion Vim's rule to act on the first pair of quotes surrounding the cursor on the current line is a good compromise. Everything else would probably result in unpredictable behavior.

Would you mind giving this PR a go and seeing where it causes unpredictable behaviour?