Selecting and exporting all lines in the buffer confusingly reports "exported the entire file"

[dbarnett]

I was troubleshooting how exporting visual ranges works and really confused to see "Syncopate exported the entire file" when I had specifically selected just part of a line (esp. since I was expecting exporting partial lines to work, #30). Would be saner to ensure it reports "entire file" only when the entire-file version was invoked.

Unfortunately, it's not possible to directly determine whether an explicit range was passed, so the current mapping/command setup with -range=% would need to be tweaked a bit. At the very least, #30 would make the visual mapping not report "entire file".

[dbarnett]

Or, simpler, just get rid of the "entire file" branch and always report the actual # of lines.

[chiphogg]

I can't reproduce this. It always reports the correct number of lines for me.

Maybe you really did export the entire file?

Feel free to reopen with instructions to reproduce if you're still seeing this.

[dbarnett]

Shouldn't be hard to reproduce. The code is directly checking a:first == 1 && a:last == line('$'). Just visual highlight from somewhere on first line to somewhere on last line and type <Leader><>.

Note I'm not saying it's "inaccurate" as such, I'm saying I'm asking it to do one thing (range export) and it's reporting that it did another (entire buffer export). But it's really confusing when I asked it to do something different (partial line export), it rounds that up to the entire buffer, then reports "exported entire buffer". It's like if I asked my GPS to navigate to an apartment complex and it answered "Navigating to John Q. SomeTenant's residence"; technically accurate, but disorienting.

[chiphogg]

Ah, I think I misread this part:

I had specifically selected just part of a line

I took that to mean your entire selection was just one part of one line, but the issue title makes it more clear what you meant.

Now that I understand the bug, we can discuss what (if anything) to do about it. :)

I'm not sure this is a problem. The behaviour of the software is different from your expectations, so it seems good for it to tell you what it actually does. It'd be nice to have different wording for visual selections, but as you point out, we can't currently tell whether we had a visual selection.

I don't really want to get rid of the "entire file" branch; I think it's useful to know.

I see two paths forward:

1. Close this as WAI.
2. Tweak the mapping/command so that we can tell when we used visual selection (and maybe even which kind of visual selection). Then we could warn users who use partial or blockwise selection that syncopate only works on whole lines.

What do you think?

[dbarnett]

Yeah, tweaking the mapping/command sounds right to me, and doable. I just mentioned dropping the "entire file" message as a fallback approach if you didn't think it was worth the effort to improve detection.

Also good idea to have custom handling for the partial selection case. It wouldn't have to be like a red error message, could just add a word or two to the existing message like "(entire lines, partial export unsupported)".

[chiphogg]

I wanted to detect visual mode at the lowest level possible, so that everything would Just Work.

mode() looks like it should give us this information. However, sticking a mode() inside the function reports n, even when we call it from visual mode. In fact, this appears to be the expected behaviour. Basically, it seems to be impossible for the function to know whether it was called from visual mode.

So, our choices are:

1. Make special visual-mode logic which only the mapping can trigger, not the command.
2. Close as WAI.

The inconsistency in the first option really bugs me. Yes, it's true that we expect the mapping to be used more than the command. But the current status quo is both consistent and correct.

The downside of the status quo is that we don't tell users we can't export partial lines, and we tell users we exported the whole file if they visually select the whole file. The second one doesn't bother me at all, and I can live with the first one.

Am I making too big a deal of the inconsistency? What do you think of our options?

[xanderman]

I've seen a common pattern of having the mapping and the command both call the same function, but passing a different value for a parameter called 'is_visual' or similar. Is that so bad here?

[chiphogg]

That's pretty much how I was planning to implement it, if we decide to do this.

The case that bugs me is when somebody uses the command from visual mode. IIUC, there's no way to make that command pass is_visual to the function if-and-only-if it was run from visual mode.

[dbarnett]

The mapping was the part I was particularly concerned about.

For the explicit command, it's not just whether you're in visual mode. It would be nice if :1,3SyncopateExportToClipboard never reported "entire buffer" even if there are only 3 lines in the buffer.