-
Notifications
You must be signed in to change notification settings - Fork 130
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
RMarkdown: Add run & navigation commands. More customization. Refactor. #465
Conversation
For this same PR, I added new commands and manual tests in my very first post above. Please re-check. Also edited the PR title.
|
Thanks for implementing numerous run chunks commands. Previously, I didn't factor the code. The CodeLens commands and extension commands thus do not share the algorithm of chunk detection but independently does the work. Now that there are many versions of run chunks, I guess we should implement a function that returns an array of the info of each chunk and a function that returns the chunk id of a given all chunks detected and a document position. Then all run chunk commands and CodeLens commands could be much simplified by using the functions. Based on this, we could also implement go-to-chunk commands such as "go to previous chunk", "go to next chunk", "go to first chunk", "go to last chunk". |
885c01b
to
1c26e83
Compare
Hi @renkun-ken, thanks for everything you have done for VSCode-R & {languageserver}. Yeah I am happy and will try my best to help out a bit for the stuff that I used everyday 😄 This is actually my first time playing around with Typescript/Javascript. I have no experience with object-oriented languages too. Happy to learn if it's not too hard. The stuffs I did here were mainly based on my experience implementing some custom Vimscript for my Neovim editor e.g. finding patterns in documents, with very almost no/minimal understanding of Typescript and internals of VSCode extensions. So just a heads up that if the refactoring is too hard then I might need some time to learn a bit of Typescript from scratch? So basically I just scanned through your Typescript implementations and playing around a bit here and there lol. So I'm actually a bit lost when you mention about CodeLens commands and extension commands, etc. I actually don't know what is a 'codelens' lol. Please bare with me while I'm trying to understand what you mean here. I just read through a bit of extension.ts and rmarkdown.ts. So the CodeLens are actually the buttons for Rmd docs e.g. Erm...So basically you want to refactor reusable parts I was actually trying to figure out what you mean...in details...an hour ago but I just thought that it might be easier to try a rough refactoring and see if that's what you wanted. Because the hard part is figuring out the chunk where the cursor is positioned and also covering the cases for when the cursor is within or outside the chunk. I just did a rough WIP commit for refactoring Based on the WIP commit, |
CodeLens commands are inline commands "Run Chunk | Run Above" showing in the document above each R chunk while extension commands are commands declared in |
I could imagine a cleaner approach is to implement something like: interface RMarkdownChunk {
id: number;
startLine: number;
endLine: number;
language: string;
options: string;
eval: boolean;
chunkRange: Range;
codeRange: Range;
}
function getChunks(document: TextDocument): RMarkdownChunk[]
function getCurrentChunk(chunks: RMarkdownChunk[], line: number): RMarkdownChunk
function getPreviousChunk(chunks: RMarkdownChunk[], line: number): RMarkdownChunk
function getNextChunk(chunks: RMarkdownChunk[], line: number): RMarkdownChunk
function runCurrentChunk(chunks: RMarkdownChunk[], line: number)
function runPreviousChunk(chunks: RMarkdownChunk[], line: number)
function runNextChunk(chunks: RMarkdownChunk[], line: number)
function runAboveChunks(chunks: RMarkdownChunk[], line: number)
function runAboveAndCurrentChunks(chunks: RMarkdownChunk[], line: number)
function runBelowChunks(chunks: RMarkdownChunk[], line: number)
function runCurrentAndBelowChunks(chunks: RMarkdownChunk[], line: number) where |
Thanks very much. That looks good. |
5bff10b
to
990bb5b
Compare
A quick update on the latest WIP: Refactor commit 990bb5b Very briefly
TODO
|
The latest WIP GIF for both extension commands and codeLens buttons
|
For |
I believe it makes good sense. |
It seems like values of Type array default settings // Default package.json
"r.rmd.codeLens.option": {
"type": "array",
"default": [
"r.runCurrentChunk",
"r.runAboveChunks",
"r.runCurrentAndBelowChunks",
"r.runAllChunks",
"r.goToPreviousChunk",
"r.goToNextChunk"
],
"description": "R RMarkdown CodeLens options.",
"items": {
"type": "string"
}
} import { config } from './util';
// This works
const rmdCodeLensOpt = ['r.runAllChunks', 'r.goToPreviousChunk'];
// Doesn't work
const rmdCodeLensOpt: string[] = config().get('r.rmd.codeLens.option');
return this.codeLenses.filter(e => rmdCodeLensOpt.includes(e.command.command)); ...which produces Similarly, for type string default settings // Default package.json
"r.rmd.codeLens.option": {
"type": "string",
"default": "r.goToNextChunk",
"description": "test"
} // This work
const rmdCodeLensOpt = 'r.runAllChunks';
// Doesn't work
const rmdCodeLensOpt: string = config().get('r.rmd.codeLens.option');
return this.codeLenses.filter(e => e.command.command === rmdCodeLensOpt); |
Oh nvm I fixed it after reading docs of Also tested that it works with both
|
Personally I turn off codelens as I mostly use keyboard. But now users who use buttons can now easily customize their preferred RMarkdown codelens buttons to show directly in their editor e.g. they can use keybindings for commonly used commands like I still can't believe VSCode is so customizable. The API is superb too. This is the package.json options {
"r.rMarkdownCodeLens.option": {
"type": "array",
"items": {
"type": "string"
},
"default": [
"r.runCurrentChunk",
"r.runAboveChunks",
"r.runCurrentAndBelowChunks",
"r.runAllChunks",
"r.goToPreviousChunk",
"r.goToNextChunk"
],
"description": "Available options:\n\nr.runCurrentChunk\nr.runAboveChunks\nr.runCurrentAndBelowChunks\nr.runBelowChunks\nr.runAllChunks\nr.runPreviousChunk\nr.runNextChunk\nr.goToPreviousChunk\nr.goToNextChunk\nr.goToFirstChunk\nr.goToLastChunk\n\n----------------------------------\nSpecify options:"
},
} |
990bb5b
to
1aafabe
Compare
Thank you for your early fix. |
For the following rmd:
The run chunks commands seem to ignore chunk language, i.e. they run chunks of all languages. Instead, they should only send R chunks to the terminal. Also, some edge cases are not properly handled yet. For example, Run Above the first chunk, or Run Below the last chunk, or select current chunk with cursor outside any chunk will cause errors like I think all these commands should handle the case where no chunk is found by simply ignoring them. |
@renkun-ken Thank you for your test, I made maintenance issues. |
The error handling can be easily fixed (i guess) ? The 'case for when cursor is below chunk', I spent few hours on it but the bug is just weird. Without being able to check the values of variables like in REPL debugging is quite hard for me. I still don't get the way of Javascript way of debugging. Need to learn a bit of it. So it might take some time to fix this particular bug. For non-R languagues bugs, I haven't looked at it yet. |
@renkun-ken I figured out the non-R languages bugs, this line https://github.com/Ikuyadeu/vscode-R/blob/3e5822b0e3bf1025e77a79aaf90e72af8db948c7/src/rmarkdown.ts#L65 only handles the CodeLens but doesn't handle the extension commands, which can be hot fixed by e.g. // Original
export async function runCurrentChunk(chunks: RMarkdownChunk[] = _getChunks(),
line: number = _getStartLine()) {
const currentChunk = getCurrentChunk(chunks, line);
runChunksInTerm([currentChunk.codeRange]);
}
export async function runCurrentChunk(chunks: RMarkdownChunk[] = _getChunks(),
line: number = _getStartLine()) {
const currentChunk = getCurrentChunk(chunks, line);
if (currentChunk.language === 'r') {
return runChunksInTerm([currentChunk.codeRange]);
}
} |
If you think CodeLens and extension commands should be separated, then we can make a helper e.g. function runChunksInTerm__R(chunk: RMarkdownChunk, range: Range) {
if (chunk.language === 'r') {
return runChunksInTerm([range]);
}
} |
This is my first time playing around with Typescript/javascript. So please review the codes in details thanks! 😅 😂 🤣
Summary
What problem did you solve?
r.rmarkdown.codeLensCommands
). See below.Also closes #262
Usage
Customize via Settings UI or put this at keybindings.json
r.selectCurrentChunk
r.runCurrentChunk
r.runAboveChunks
r.runCurrentAndBelowChunks
r.runBelowChunks
r.runPreviousChunk
r.runNextChunk
r.runAllChunks
r.goToPreviousChunk
r.goToNextChunk
Animation for both
(Click to enlarge)
Images for User-specified RMarkdown code lens commands and its orders (that is, code lens respect user-specified orders)
Customize options and order (code lens respect user-specified orders) via settings UI (RMarkdown Code Lens: Option) or settings.json (
r.rMarkdownCodeLens.option
).More customizations
User-friendly message for new users
Enable RMarkdown CodeLens (default)
"r.rmarkdown.enableCodeLens": true
(default)Disable RMarkdown CodeLens
"r.rmarkdown.enableCodeLens": false
Customize chunk background colors
r.rmarkdown.chunkBackgroundColor: "rgba(128, 128, 128, 0.1)",
r.rmarkdown.chunkBackgroundColor: "rgba(255, 165, 0, 0.1)",
Disable background color
r.rmarkdown.chunkBackgroundColor: "",
Use cases for both
r.runFromCurrentToBelowChunks
&r.runBelowChunks
r.runCurrentChunk
for checking values/etc and, when done,r.runBelowChunks
from the next chunk (excluding the current chunk) to all chunks belowr.runFromCurrentToBelowChunks
from the current chunk to all chunks belowDesign decisions & known bugs
By design, these commands work for both cases when the cursor is
Case 'within the chunk'
Works as expected
Case cursor outside of chunk
The reason for including cases for 'outside the chunk' is to make it work for
goToPreviousChunk
,goToNextChunk
,goToFirstChunk
,goToLastChunk
, which is very convenient especially for large Rmd files with many chunks and a lot of text in between chunks.runCurrentAndBelowChunks
) and to make the behaviour consistent with navigation commandsAs a result, when the cursor is outside the chunk, the definition of the 'current chunk' and 'next chunk' become blur, that is, there are no 'correct' definitions, simply because there is no definition of 'current chunk'. To make it makes more sense, when cursor is outside the chunk, for the reason that navigation commands (e.g.
goToNextChunk
) is more useful than run commands (e.g.runCurrentAndBelowChunks
), by design, the definition of the 'current chunk' and 'next chunk' are the same--the very next chunk. That is, e.g.goToNextChunk
,runCurrentChunk
andrunNextChunk
trigger the very next chunkrunCurrentAndBelowChunks
andrunBelowChunks
trigger from the very next chunk to the last chunkCompared to the case where
goToNextChunk
will jump to next next chunk when cursor is outside the chunk, which doesn't make much sense. And it also doesn't make sense to make another command forgoToCurrentChunk
just for this trade-off.So it's a trade-off for its design. I think the overall benefits e.g.
goToNextChunk
,goToFirstChunk
outweigh the minor 'unexpected' behaviours when the cursor is outside the chunk, especially for large Rmd files with many chunks and a lot of text in between the chunks. Code navigation is often the most time consuming part. e.g.goToPreviousChunk
often saves a lot of timeLastly, a known bug is when the cursor is below the very last chunk,
goToFirstChunk
,goToLastChunk
,runAllChunks
work as expected (in all cases) becauses these commands doesn't require the 'current line' as argument so the cursor position doesn't affect it at all.goToPreviousChunk
,runPreviousChunk
andrunAboveChunks
I thought the bugs is in internal functiongetCurrentChunk
but I spent some time on it there is still some bug. I will come back and work in near future if that's okay. The bug is just weird.Tested on Windows WSL remote extension