Skip to content
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

R code cell parser issues #3709

Closed
kylebutts opened this issue Jun 28, 2024 · 5 comments
Closed

R code cell parser issues #3709

kylebutts opened this issue Jun 28, 2024 · 5 comments
Labels
area: code cells Issues related to the Code Cells extension bug Something isn't working support

Comments

@kylebutts
Copy link
Contributor

Positron Version:

Positron-2024.06.1-27

Steps to reproduce the issue:

My favorite part of VSCode and R is the code cell feature, so I'm very excited to see support in Positron. There's a few features missing, so I wanted to flag them here. I'm happy to submit a pull request if outside contributions are welcomed? I've contributed to the R VSCode on this feature, so am aware of VSCode extension development (REditorSupport/vscode-R#1454)

  1. Create a new R file and a new python file and insert this code:
#+
2 + 1
3 + 2 


#+ 
1 + 1

2 + 2

#+
2 + 2

#' # Hi
#' 
#' Testing `knitr::spin` markdown syntax
#' 
# %%
2 + 1
3 + 2


# %%
1 + 1

2 + 2

# %%
2 + 2
# %% [markdown]
# Hi

Testing `knitr::spin` markdown syntax

What did you expect to happen?

This is the resulting view:
CleanShot 2024-06-28 at 12 57 50@2x

Here are the bugs I'm seeing

  1. Note the cells only can be one contiguous blob of code (no new lines). That is from isCellEnd: (line) => line.trim() === '',. I think the tests for this have no extra new lines, so this is not noticed.

    const rCellParser: CellParser = {
    isCellStart: (line) => line.startsWith('#+'),
    isCellEnd: (line) => line.trim() === '',
    getCellType: (_line) => CellType.Code,
    getCellText: getCellText,
    newCell: () => '\n\n#+',
    cellDecorationSetting: () => CellDecorationSetting.All,
    };

  2. Cold folding does not seem to work (it does in python). I'm not sure how this test is passing. When I copy the text to mine, it does not offer folding.

    const content = `#+
    testing1
    #+
    testing2
    #+
    testing3`;

  3. Pyright is creating errors for a markdown cell.

  4. knitr has markdown support via the #' syntax, but is not being recognized

  5. Also, # %% is supported by VSCode, knitr::spin(), and quarto. Would love to add that option for R: https://www.rdocumentation.org/packages/knitr/versions/1.46/topics/spin

Were there any error messages in the output or Developer Tools console?

There were not any error messages.

@kylebutts kylebutts added the bug Something isn't working label Jun 28, 2024
@jmcphers
Copy link
Collaborator

Thanks for the feedback!

Cold folding does not seem to work (it does in python). I'm not sure how this test is passing. When I copy the text to mine, it does not offer folding.

This is #2924; we've temporarily disabled the code cell folder (which indeed used to work) until we can integrate it with the other language folding rule providers.

@seeM
Copy link
Contributor

seeM commented Jun 30, 2024

Thanks Kyle! Aside from the issue Jonathan mentioned, the rest of your points do indeed look like bugs. Outside contributions are welcomed, please feel free to tag me in any PRs.

@juliasilge juliasilge added the area: code cells Issues related to the Code Cells extension label Jun 30, 2024
@kylebutts
Copy link
Contributor Author

Hi @seeM, I'm working on a PR and wanted to add more test coverage. I'm trying to figure out how to run the tests with mocha. What is the correct way to do that? Compile to out/ and then run mocha out/?

@seeM
Copy link
Contributor

seeM commented Jul 3, 2024

I usually:

  1. Run yarn from the root positron dir.
  2. Run both yarn watch-client and yarn watch-extensions from the same dir to compile to TS on save.
  3. Run yarn test-extension -l positron-code-cells from the same dir to test the code cells extension specifically. You can additionally pass -g <grep> to grep for specific test suites/cases.

Steps 1 and 2 are both needed since these tests actually run against a build of Positron.

The VSCode contributing docs may also be helpful: https://github.com/microsoft/vscode/wiki/How-to-Contribute.

kylebutts added a commit to kylebutts/positron that referenced this issue Jul 3, 2024
- R parser would end cells after first blank line; this is now removed
- `# %%` is now recognized as a valid cell demarcation in R
- cellDecorationSetting now matches python (only highlights currently active cell
- Tests added to test for these changes
@juliasilge juliasilge added this to the 2024.08.0 Pre-Release milestone Jul 5, 2024
seeM added a commit that referenced this issue Jul 8, 2024
Addresses #3709.

Changes:
- R parser would end cells after first blank line; this is now removed
- `# %%` is now recognized as a valid cell demarcation in R
- cellDecorationSetting now matches python (only highlights currently
active cell). Note that before the whole program is highlighted a gray
color, so the information was meaningless.
- Tests added to test for these changes

---------

Signed-off-by: Kyle F Butts <buttskyle96@gmail.com>
Co-authored-by: Wasim Lorgat <mwlorgat@gmail.com>
@jonvanausdeln
Copy link
Contributor

Verified Fixed

Positron Version(s) : 2024.07.0-67
OS Version(s) : Windows 11

Test scenario(s)

Provided examples work as expected

Link(s) to TestRail test cases run or created:

Tests were included as part of PR

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: code cells Issues related to the Code Cells extension bug Something isn't working support
Projects
None yet
Development

No branches or pull requests

5 participants