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

adding support for didact extension #459

Merged
merged 11 commits into from
Jul 7, 2020

Conversation

bfitzpat
Copy link
Contributor

@bfitzpat bfitzpat commented May 6, 2020

Signed-off-by: bfitzpat@redhat.com bfitzpat@redhat.com

What does this PR do?

TODO

Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented May 6, 2020

Can one of the admins verify this patch?

1 similar comment
@che-bot
Copy link
Contributor

che-bot commented May 6, 2020

Can one of the admins verify this patch?

@ericwill
Copy link
Contributor

ericwill commented May 6, 2020

Thanks for the PR! Does this plugin have any particular dependencies?

@bfitzpat
Copy link
Contributor Author

bfitzpat commented May 6, 2020

Hey folks. :)
Not too many dependencies. This relies very heavily on the vscode API for commands though.
Here's the dependencies list - https://github.com/redhat-developer/vscode-didact/blob/e4e04b7d4e8b0f163524eea1bbbae41227a15ca7/package.json#L179 - it's pretty minimal.
But we use vscode.commands.executeCommand() heavily (see https://github.com/redhat-developer/vscode-didact/blob/e4e04b7d4e8b0f163524eea1bbbae41227a15ca7/src/commandHandler.ts#L214)
Essentially didact merges some simple text (markdown or asciidoc) and provides an interactive link that calls into the commands in vscode.

Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
@bfitzpat
Copy link
Contributor Author

bfitzpat commented May 6, 2020

Mistyped -- that's really moving from redhat-developer to redhat (as the folder names)

@bfitzpat
Copy link
Contributor Author

bfitzpat commented May 6, 2020

There are a few issues to resolve. I suspect that the CSP on the HTML loaded into the webview isn't being handled by Theia/Che right now. None of our stylesheet changes are loaded and it doesn't seem that our JavaScript processing links and passing messages back to vscode is operating.

Hopefully we can figure that bit out. But this is a good first step. :)

Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
@bfitzpat
Copy link
Contributor Author

bfitzpat commented May 7, 2020

After removing the container declaration (thank you @azatsarynnyy ), we got things running. There are still some things to figure out as far as command execution (some commands or views are missing like workbench.view.explorer). So I'm working my way through that effort now.

Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
@bfitzpat
Copy link
Contributor Author

So in doing the testing for the Didact plug-in, we use several standard VS Code-provided commands...
It looks like looks explorer.newFolder and workbench.view.explorer commands are not available in Che. Any chance we can get them added? @azatsarynnyy ?

@bfitzpat
Copy link
Contributor Author

bfitzpat commented May 14, 2020

Other missing commands include:

  • workbench.action.terminal.sendSequence
  • workbench.action.terminal.kill
  • copyFilePath

@azatsarynnyy Here are the three others we use. Beyond this we seem to be ok for the most part.

How difficult is it to migrate commands from vscode to theia?

@azatsarynnyy
Copy link
Member

@bfitzpat we're already working on adding support for VS Code commands in Theia. I've added the missing ones for Didact extension to the epic eclipse-theia/theia#4050
We'll take them with a higher priority. Thanks!

Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
@azatsarynnyy
Copy link
Member

Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
@bfitzpat bfitzpat marked this pull request as ready for review June 23, 2020 12:58
@bfitzpat bfitzpat requested a review from apupier June 23, 2020 12:58
Copy link
Contributor

@apupier apupier left a comment

Choose a reason for hiding this comment

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

first feedback before testing the extension

v3/plugins/redhat/vscode-didact/0.1.15/meta.yaml Outdated Show resolved Hide resolved
v3/plugins/redhat/vscode-didact/0.1.15/meta.yaml Outdated Show resolved Hide resolved
Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
@benoitf
Copy link
Contributor

benoitf commented Jul 2, 2020

@apupier are you still missing some changes ? as it's in 'requested changes' mode

@apupier
Copy link
Contributor

apupier commented Jul 2, 2020

are you still missing some changes ? as it's in 'requested changes' mode

not that I'm aware of.
"Just" need that someone try with latest version to check that it works as expected and doesn't reveal other things by going further in the manual tests

@apupier apupier self-requested a review July 2, 2020 15:51
@bfitzpat
Copy link
Contributor Author

bfitzpat commented Jul 2, 2020

As of right now, che.openshift.io is still on eclipse/che-theia/7.14.3 - Eclipse Theia. I believe the changes we need are in 7.15, so I keep checking at che.openshift.io to see when 7.15 is available for use. Checking every few days, it hasn't rolled up to 7.15 yet.

@ericwill
Copy link
Contributor

ericwill commented Jul 2, 2020

As of right now, che.openshift.io is still on eclipse/che-theia/7.14.3 - Eclipse Theia. I believe the changes we need are in 7.15, so I keep checking at che.openshift.io to see when 7.15 is available for use. Checking every few days, it hasn't rolled up to 7.15 yet.

I think you can expect che.openshift.io to have the 7.15 bits sometime next week, @ibuziuk would know better though.

Copy link
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

tested with https://che.openshift.io/f/?url=https://gist.githubusercontent.com/benoitf/c219189e9b70626aa99703d4978944a4/raw/926054bde70e791d15aca829cb54a62e399ade3d/didact-devfile.yaml

One thing that is not working is the open file as it seems it's more opening a workspace than a file
but it's minor for not accepting it

vscode-didact

@benoitf
Copy link
Contributor

benoitf commented Jul 3, 2020

I've relaunched circle-ci build that failed for a system error

@sunix
Copy link
Contributor

sunix commented Jul 3, 2020

hey so also tested :) this is really good. Some feedback:

  • in che, didact will run terminal and send text to terminal OK it works. But in a che workspace, users usually run terminal in dedicated container, not the theia-ide container. So for instance, in a tutorial, if i say: open the terminal in the centos-maven container, and run mvn clean install, it should work in my opinion. Would it be worth it that we include in that Che plugin, extensions with che specific commands ? to be able to play the terminal example but in the context of a real case Che workspace?
  • tried to open a file with vscode.open, but it failed ... I suspect the string is not uri. https://github.com/eclipse-theia/theia/blob/master/packages/plugin-ext-vscode/src/browser/plugin-vscode-commands-contribution.ts#L104
  • I am looking at the command id to run a task by name ... any idea?

@apupier apupier dismissed their stale review July 7, 2020 08:45

my requested change has been updated. Dismissing as i have not reviewed again.

@benoitf benoitf merged commit 7b1d83a into eclipse-che:master Jul 7, 2020
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.

7 participants