Skip to content

Implement R workspace viewer#476

Merged
renkun-ken merged 17 commits intoREditorSupport:masterfrom
ElianHugh:master
Dec 8, 2020
Merged

Implement R workspace viewer#476
renkun-ken merged 17 commits intoREditorSupport:masterfrom
ElianHugh:master

Conversation

@ElianHugh
Copy link
Copy Markdown
Collaborator

@ElianHugh ElianHugh commented Nov 29, 2020

What problem did you solve?

Closes #416, implementing a workspace/environment viewer.

Screenshot

image

How can I check this pull request?

What should happen:

  1. The workspace viewer is available on the activity bar
  2. When the global environment is updated, the viewer is automatically refreshed
  3. Clicking the "View" button on a variable should call View() in the R terminal.
  4. Clicking the Clear all button should call rm(list = ls()) in the R terminal.

Changelog

c3f3c17

  • Fixed redundant view title
  • Added remove command
  • Simplified workspace.ts

2a97b29

  • Implemented save/load workspace buttons
  • Fixed typings

7b4edd4

  • Implemented confirmation box for workspace clearing
  • Changed default folder for save/load dialog

8c86577

  • Implemented element sorting for the workspace viewer
    • Sort priority is: data variables (data.frame, list, environment, etc.) -> alphabetical label sort -> alphabetical class sort
  • Added settings option for removing hidden items
  • Changed default folder for saving/opening .RData files (subject to change)
  • Renamed workspace.ts -> workspaceViewer.ts
  • Fixed some typings

44f0b79

  • Changed getWorkspacePaths() to workingDir

9e2eef3

  • Changed save string from 'workspace' to 'workspace.RData' due to save filters not applying on non-Windows OS

fd06ed0

  • Changed save string from concatenation to template literal
    • defaultUri: Uri.file(workingDir + path.sep + 'workspace.RData') to defaultUri: Uri.file(${workingDir}${path.sep}workspace.RData)

dd912c9

  • Changed the sort function from ternary form to if-else form

cf1ef1f

  • Changed the priority list for sorting from variable classes (e.g. data.frame) to variable types (e.g. list)

d9eb94c

  • Added globalenv conditional for workspace commands, so that they can't be run if session isn't attached

2bae9c0

  • Changed if statements from equality (==) to identity (===)

6cc95e8

  • Use dim() as description for eligible variables
  • Fix double quote usage

4036a19

  • Fix lint errors
  • Fix matrix and array descriptions

f822da8

  • Dim() only applies to list-type data structures
  • Fixed typo pertaining to dim() descriptions with only 1 variable

Removed superflous code, changed import, changed formatting
Comment thread package.json Outdated
@renkun-ken
Copy link
Copy Markdown
Member

renkun-ken commented Nov 29, 2020

One thing to notice is that when an R session exits, the globalenv.json is removed, but we don't handle such case at the moment, and then the workspace view does not reset. We'll probably fix this in a separate PR later.

Comment thread package.json
Comment thread src/extension.ts Outdated
Copy link
Copy Markdown
Member

@ManuelHentschel ManuelHentschel left a comment

Choose a reason for hiding this comment

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

Thanks for the nice work! :)

Comment thread src/workspace.ts Outdated
Comment thread src/workspace.ts Outdated
Comment thread src/workspace.ts Outdated
Implementing suggestions of PR 476
@renkun-ken
Copy link
Copy Markdown
Member

In RStudio, the environment pane has Load and Save buttons so that the user could click to load a workspace from or save one to .RData file.

image

It seems we could do similar by sending load(file) and save.image(file) to do the same.

Implementing suggestions of PR 476. Save/import workspace buttons.
Fixed some typing issues.
@ElianHugh
Copy link
Copy Markdown
Collaborator Author

In RStudio, the environment pane has Load and Save buttons so that the user could click to load a workspace from or save one to .RData file.

image

It seems we could do similar by sending load(file) and save.image(file) to do the same.

As per your suggestion, 2a97b29 implements save/load workspace buttons.

Comment thread src/workspace.ts Outdated
Comment thread src/workspace.ts Outdated
Implement confirmation box for clearing environment. Implemented getWorkspacePath(), which returns the current workspace folder.
Comment thread src/workspace.ts Outdated
@ManuelHentschel
Copy link
Copy Markdown
Member

Maybe we should rename the file workspace.ts to e.g. workspaceViewer.ts to be more precise about what is implemented in the file

Rename workspace to workspaceViewer, add settings option for removing hidden items, sort elements in workspace viewer, fixed some typings
Use workingDir instead of getWorkspacePath()
Comment thread src/workspaceViewer.ts Outdated
Comment thread src/workspaceViewer.ts Outdated
Change sort function from ternary form to if-else form
Comment thread src/workspaceViewer.ts
@ElianHugh
Copy link
Copy Markdown
Collaborator Author

I've noticed that open/save workspace require the R session to be attached for them to work. Is there a way to force attach or require attach?

@renkun-ken
Copy link
Copy Markdown
Member

I've noticed that open/save workspace require the R session to be attached for them to work. Is there a way to force attach or require attach?

If r.alwaysUseActiveTerminal = false (default), then an R terminal is started if there's no active terminal. Otherwise, it will directly send text to the current terminal (if any) no matter whether there's an open terminal.

Currently, I suggest that you check if globalenv is undefined before sending commands to the terminal. In the future, we'll fix this by resetting globalenv to undefined if the active R session is terminated.

Don't run workspace viewer commands unless globalenv is defined
Comment thread src/workspaceViewer.ts Outdated
Comment thread src/workspaceViewer.ts Outdated
Use dim() where possible for data.frame-like element descriptions
@ElianHugh
Copy link
Copy Markdown
Collaborator Author

ElianHugh commented Dec 6, 2020

Last commit allows for more RStudio-esque data descriptions. Instead of relying just on str(), we use dim() to get the number of observations for eligible variables

image

This means things like tibbles have better descriptions :)

Comment thread src/workspaceViewer.ts Outdated
Comment thread src/workspaceViewer.ts Outdated
@renkun-ken
Copy link
Copy Markdown
Member

@ElianHugh Thanks for the updated. Please notice and fix the eslint complains in the diffs.

@ElianHugh
Copy link
Copy Markdown
Collaborator Author

@ElianHugh Thanks for the updated. Please notice and fix the eslint complains in the diffs.

Will do! For some reason my local eslint isn't picking up some of these errors...

Comment thread src/workspaceViewer.ts
@ElianHugh
Copy link
Copy Markdown
Collaborator Author

ElianHugh commented Dec 7, 2020

I think we should make a decision re: which data types use the ? obs. format. Currently we have:

  • List-type data structures
    • e.g. data.frame, tbl, data.table
  • Array-type
    • e.g. simple array(), matrices

This leaves some data types potentially unaccounted for:

  • vectors created through c()
  • vectors created through x:y
    • eg. vec <- 1:10
  • factors

From what I can tell, RStudio seems to only treat list-type data structures as the ? obs. format. This means that matrices and arrays are shown as, for example num [1:2, 1] 1 1.

Edit:

Oops, just saw your post!

Comment thread R/init.R
Fixed a typo in getDescription, dim() only applied to list-type data structures
Copy link
Copy Markdown
Member

@renkun-ken renkun-ken left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for your contribution to this feature!

@Ikuyadeu @andycraig @ManuelHentschel Any more comments?

@andycraig
Copy link
Copy Markdown
Collaborator

@renkun-ken I won't have time to look at this for a while, so please proceed without me.

@renkun-ken
Copy link
Copy Markdown
Member

@ElianHugh the workspace viewer relies on session watcher to work. Therefore, I guess we don't need to create workspace viewer if session watcher is disabled? Not sure if it is possible, but an alternative way is to tell user one must enable session watcher to use the feature in the default text if it finds that session watcher is disabled?

@Ikuyadeu
Copy link
Copy Markdown
Member

Ikuyadeu commented Dec 8, 2020

@renkun-ken LGTM
After squashing this PR, I think we need to refactor the command category like the python extension.

@renkun-ken renkun-ken merged commit 4de62eb into REditorSupport:master Dec 8, 2020
@ElianHugh
Copy link
Copy Markdown
Collaborator Author

LGTM

Thanks for your contribution to this feature!

@Ikuyadeu @andycraig @ManuelHentschel Any more comments?

Great! Thank you very much for help and suggestions!

@ElianHugh the workspace viewer relies on session watcher to work. Therefore, I guess we don't need to create workspace viewer if session watcher is disabled? Not sure if it is possible, but an alternative way is to tell user one must enable session watcher to use the feature in the default text if it finds that session watcher is disabled?

Hmm, that's a good point. I think it should be possible to hide the viewer when session watcher is disabled via the package.json file (when property). I'll try it out in a little bit.

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.

Workspace viewer similar to vscode-julia

5 participants