rstudio / rstudioapi

Safely access RStudio's API (when available)
http://rstudio.github.io/rstudioapi
Other
165 stars 35 forks source link

insertText()'s location= argument -- is '0' a valid column number? #299

Open MichaelChirico opened 2 months ago

MichaelChirico commented 2 months ago

https://github.com/rstudio/rstudioapi/blob/3304988af6ba2b70ec1774aa6eb31a37a75d4c7e/R/document-api.R#L33

I couldn't tell from ?insertText whether 0 is valid or not. Of course I could test this with RStudio available, but I don't have it available at the moment, so it would be good to have this written out. The source code of rstudioapi::insertText() also eventually refers to a function inside an environment only available inside RStudio, so I couldn't check that either:

https://github.com/rstudio/rstudioapi/blob/3304988af6ba2b70ec1774aa6eb31a37a75d4c7e/R/code.R#L156

Context: We have a conflict in {lintr} of 0 being a valid column number for lint objects, apparently to support RStudio:

https://github.com/r-lib/lintr/pull/2418#discussion_r1555250719

But this conflicts with SARIF format (https://github.com/r-lib/lintr/issues/2550), which requires column>0. I'd like to know if the fix is to block 0 as a column number everywhere, or if we should just block it when producing SARIF output.

To answer this I tried to refer to the {rstudioapi} documentation, but to no avail.

kevinushey commented 2 months ago

It's allowed, but RStudio / rstudioapi used one-based indexing for document offsets, so inserting something at column 0 is the same as inserting it at column 1. (Same for row 0 versus row 1).

MichaelChirico commented 2 months ago

OK, great. is it safe to say the same applies for our linting add-in? it will land the cursor on a column number, is there any difference for 0 vs 1 in that case (sorry, only now realizing focusing on insertText doesn't necessarily fully answer my underlying question)