Skip to content

extendSelection only handles brackets outside quotes#314

Merged
renkun-ken merged 6 commits intoREditorSupport:masterfrom
renkun-ken:extend-selection-quotes
May 10, 2020
Merged

extendSelection only handles brackets outside quotes#314
renkun-ken merged 6 commits intoREditorSupport:masterfrom
renkun-ken:extend-selection-quotes

Conversation

@renkun-ken
Copy link
Copy Markdown
Member

What problem did you solve?

Closes #173

This PR enhances extendSelection so that it could handle string and symbol enclosed between quotes (", ', `). Raw string is not yet handled due to its complexity. This should be working for most cases at the moment.

(If you do not have screenshot) How can I check this pull request?

Put cursor at the first and last line of each code chunk and send it to terminal via extendSelection, and all should work and send code correctly.

lapply(1:5, function(i) {
  paste0("[[", i)
})

`[[`(
  c(1, 2, 3),
  3
)

`[.test` <- function(x, i) {
  x[i]
}

lapply(1:5, function(i) {
  paste0("\"", i)
})

lapply(1:5, function(i) {
  paste0('"', i)
})

lapply(1:5, function(i) {
  paste0('\"\'', i)
})

print("
  # hello
  [ is a function
  `[[` is also a function"
)

print("\"hello"    
)

The following example should send the same code to terminal when cursor is at any line from 2 to 4.

library(magrittr)
list(x = 1,
  y = "[") %>%
  print()

@andycraig
Copy link
Copy Markdown
Collaborator

Nice work! Can you add unit tests for those examples?

@renkun-ken
Copy link
Copy Markdown
Member Author

renkun-ken commented May 6, 2020

@andycraig I'm wondering how I could run the test with mocha? I tried

npm test

> r@1.3.0 test /Users/ken/Workspaces/github/vscode-R
> echo 'Error: no test specified'

Error: no test specified

but failed.

I also tried adding

"scripts": {
  "test": "mocha"
},

to package.json and run npm test:

npm test

> r@1.3.0 test /Users/ken/Workspaces/github/vscode-R
> mocha

Error: No test files found
npm ERR! Test failed.  See above for more details.

@andycraig
Copy link
Copy Markdown
Collaborator

At some point some of the test API was moved into vscode-test - maybe some things need to be updated. I haven’t run the unit tests in a while so they may not work as-is at the moment.

I will have a look this evening.

@renkun-ken
Copy link
Copy Markdown
Member Author

Once the test works, we could add tests to GitHub actions so that it could run regularly.

@renkun-ken
Copy link
Copy Markdown
Member Author

renkun-ken commented May 7, 2020

The currently known limitation is

  1. It does not work when the line starts inside a multi-line string due to the complexity of identical open/close quotes that do not suggest looking forward/backward directions:
print("
  # hello
  [ is a function
  `[[` is also a function
") |
  1. It does not work with raw strings that contain unmatched brackets enclosed with its open/close quotes due to the complexity of looking backward for a raw string:
R"("hello", 'world')"
r"("hello", 'world')"
r"--("hello", 'world')--"
r'--("hello", 'world')--'
r'--("hello", 'world')--")--'

it works with

print(r"("hello", 'world')"
)

print(r"([)"
)

but not

print(r"("[")"
)

I don't think I'll fix these limitations at the moment as it'll add much complexity to the code and the edge cases seems to rarely occur in practice.

@andycraig
Copy link
Copy Markdown
Collaborator

@renkun-ken This PR and #316 modify some of the same files. If it's okay with you, I would like to have #316 merged first, and then we can confirm that the unit tests all pass for this PR before merging.

@renkun-ken
Copy link
Copy Markdown
Member Author

renkun-ken commented May 9, 2020

@andycraig Thanks for the good work on testing. I'm totally okay with merging #316 first. Then I'll take a look and maybe rebase this PR onto the latest master.

@renkun-ken renkun-ken force-pushed the extend-selection-quotes branch from 9a42472 to 38954bb Compare May 9, 2020 12:40
@renkun-ken
Copy link
Copy Markdown
Member Author

Rebased to #317, fixed some test cases, and everything looks good now.

@andycraig
Copy link
Copy Markdown
Collaborator

Great! I'll review tomorrow.

Copy link
Copy Markdown
Collaborator

@andycraig andycraig left a comment

Choose a reason for hiding this comment

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

LGTM.

@renkun-ken renkun-ken merged commit 725c1a8 into REditorSupport:master May 10, 2020
@renkun-ken
Copy link
Copy Markdown
Member Author

@andycraig Thanks for your review!

@andycraig
Copy link
Copy Markdown
Collaborator

@renkun-ken Thank you very much for adding this feature!

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.

Extend selection does not work when code contains brackets in quotes

2 participants