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

Support refactoring documentation #1334

Merged
merged 2 commits into from
Jul 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
623 changes: 623 additions & 0 deletions document/_java.learnMoreAboutRefactorings.md

Large diffs are not rendered by default.

Binary file added document/refactoring_menu.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 10 additions & 10 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"preview": true,
"enableProposedApi": false,
"engines": {
"vscode": "^1.44.0"
"vscode": "^1.47.0"
},
"repository": {
"type": "git",
Expand Down Expand Up @@ -781,7 +781,7 @@
"@types/lodash.findindex": "^4.6.6",
"@types/mocha": "^5.2.5",
"@types/node": "^8.10.51",
"@types/vscode": "^1.44.0",
"@types/vscode": "^1.47.0",
"@types/winston": "^2.4.4",
"gulp": "^4.0.0",
"gulp-decompress": "2.0.1",
Expand Down
40 changes: 40 additions & 0 deletions src/codeActionProvider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { CodeActionProvider, CodeActionProviderMetadata, CodeActionKind } from "vscode";
import { Commands } from "./commands";

/**
* Mapping the refactoring kind to its section id in the document
*/
export const javaRefactorKinds: Map<CodeActionKind, string> = new Map([
[CodeActionKind.Refactor, 'java-refactoring'],
[CodeActionKind.RefactorExtract, 'extract-to-constant'],
[CodeActionKind.RefactorExtract.append('function'), 'extract-to-method'],
[CodeActionKind.RefactorExtract.append('constant'), 'extract-to-constant'],
[CodeActionKind.RefactorExtract.append('variable'), 'extract-to-local-variable'],
[CodeActionKind.RefactorExtract.append('field'), 'extract-to-field'],
[CodeActionKind.RefactorInline, 'inline-constant'],
[CodeActionKind.Refactor.append('move'), 'move'],
[CodeActionKind.Refactor.append('assign'), 'assign-to-variable'],
[CodeActionKind.Refactor.append('introduce').append('parameter'), 'introduce-parameter']
]);

export class RefactorDocumentProvider implements CodeActionProvider {
provideCodeActions() {
return [];
}

public static readonly metadata: CodeActionProviderMetadata = {
providedCodeActionKinds: [
CodeActionKind.Refactor
],
documentation: Array.from(javaRefactorKinds.keys()).map(kind => {
return {
kind,
command: {
command: Commands.LEARN_MORE_ABOUT_REFACTORING,
title: 'Learn more about Java refactorings...',
arguments: [kind]
}
};
}),
};
}
2 changes: 2 additions & 0 deletions src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,4 +204,6 @@ export namespace Commands {
* Command to switch between standard mode and lightweight mode.
*/
export const SWITCH_SERVER_MODE = 'java.server.mode.switch';

export const LEARN_MORE_ABOUT_REFACTORING = '_java.learnMoreAboutRefactorings';
}
100 changes: 100 additions & 0 deletions src/markdownPreviewProvider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { Disposable, WebviewPanel, window, ViewColumn, commands, Uri, Webview, ExtensionContext, env } from "vscode";
import * as fse from 'fs-extra';
import * as path from 'path';

class MarkdownPreviewProvider implements Disposable {
private panel: WebviewPanel | undefined;
private disposables: Disposable[] = [];

public async show(markdownFilePath: string, title: string, section: string, context: ExtensionContext): Promise<void> {
if (!this.panel) {
this.panel = window.createWebviewPanel('java.markdownPreview', title, ViewColumn.Active, {
localResourceRoots: [
Uri.file(path.join(context.extensionPath, 'webview-resources')),
Uri.file(path.dirname(markdownFilePath)),
],
retainContextWhenHidden: true,
enableFindWidget: true,
enableScripts: true,
});
}

this.disposables.push(this.panel.onDidDispose(() => {
this.panel = undefined;
}));

this.panel.iconPath = Uri.file(path.join(context.extensionPath, 'icons', 'icon128.png'));
this.panel.webview.html = await this.getHtmlContent(this.panel.webview, markdownFilePath, section, context);

this.panel.reveal(this.panel.viewColumn);
}

public dispose(): void {
if (this.panel) {
this.panel.dispose();
}
for (const disposable of this.disposables) {
disposable.dispose();
}
}

protected async getHtmlContent(webview: Webview, markdownFilePath: string, section: string, context: ExtensionContext): Promise<string> {
const nonce: string = this.getNonce();
const styles: string = this.getStyles(webview, context);
let markdownString: string = await fse.readFile(markdownFilePath, 'utf8');
markdownString = markdownString.replace(/__VSCODE_ENV_APPNAME_PLACEHOLDER__/, env.appName);
const body: string = await commands.executeCommand('markdown.api.render', markdownString);
return `
<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Security-Policy" content="default-src 'none'; style-src ${webview.cspSource}; img-src 'self' ${webview.cspSource} https: data:; script-src 'nonce-${nonce}';"/>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
${styles}
<base href="${webview.asWebviewUri(Uri.file(markdownFilePath))}">
</head>
<body class="vscode-body scrollBeyondLastLine wordWrap showEditorSelection">
${body}
<button class="btn floating-bottom-right" id="back-to-top-btn">
<svg width="16" height="16" viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg">
<path fill-rule="evenodd" clip-rule="evenodd" d="M8 6.04042L3.02022 11.0202L2.31311 10.3131L7.64644 4.97976L8.35355 4.97976L13.6869 10.3131L12.9798 11.0202L8 6.04042Z"/>
</svg>
</button>
<script nonce="${nonce}">
(function() {
var element = document.querySelector('[id^="${section}"]');
if (element) {
element.scrollIntoView(true);
}
var backToTopBtn = document.getElementById('back-to-top-btn');
if (backToTopBtn) {
backToTopBtn.onclick = () => document.documentElement.scrollTop = 0;
}
})();
</script>
</body>
</html>
`;
}

protected getStyles(webview: Webview, context: ExtensionContext): string {
const styles: Uri[] = [
Uri.file(path.join(context.extensionPath, 'webview-resources', 'highlight.css')),
Uri.file(path.join(context.extensionPath, 'webview-resources', 'markdown.css')),
Uri.file(path.join(context.extensionPath, 'webview-resources', 'document.css')),
];
return styles.map((styleUri: Uri) => `<link rel="stylesheet" type="text/css" href="${webview.asWebviewUri(styleUri).toString()}">`).join('\n');
}

private getNonce(): string {
let text = "";
const possible = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
for (let i = 0; i < 32; i++) {
text += possible.charAt(Math.floor(Math.random() * possible.length));
}
return text;
}
}

export const markdownPreviewProvider: MarkdownPreviewProvider = new MarkdownPreviewProvider();
12 changes: 11 additions & 1 deletion src/standardLanguageClient.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

import { ExtensionContext, window, StatusBarAlignment, workspace, commands, Uri, ProgressLocation, ViewColumn, EventEmitter, extensions } from "vscode";
import { ExtensionContext, window, workspace, commands, Uri, ProgressLocation, ViewColumn, EventEmitter, extensions, languages, CodeActionKind } from "vscode";
import { Commands } from "./commands";
import { serverStatus, ServerStatusKind } from "./serverStatus";
import { prepareExecutable, awaitServerConnection } from "./javaServerStarter";
Expand All @@ -12,6 +12,7 @@ import { onExtensionChange } from "./plugin";
import { serverTaskPresenter } from "./serverTaskPresenter";
import { RequirementsData } from "./requirements";
import * as net from 'net';
import * as path from 'path';
import { getJavaConfiguration } from "./utils";
import { logger } from "./log";
import * as buildPath from './buildpath';
Expand All @@ -23,6 +24,8 @@ import { apiManager } from "./apiManager";
import { ExtensionAPI, ClientStatus } from "./extension.api";
import { serverStatusBarProvider } from "./serverStatusBarProvider";
import * as fileEventHandler from './fileEventHandler';
import { markdownPreviewProvider } from "./markdownPreviewProvider";
import { RefactorDocumentProvider, javaRefactorKinds } from "./codeActionProvider";

const extensionName = 'Language Support for Java';
const GRADLE_CHECKSUM = "gradle/checksum/prompt";
Expand Down Expand Up @@ -295,6 +298,13 @@ export class StandardLanguageClient {
});
}
excludeProjectSettingsFiles();

context.subscriptions.push(markdownPreviewProvider);
context.subscriptions.push(languages.registerCodeActionsProvider({ scheme: 'file', language: 'java' }, new RefactorDocumentProvider(), RefactorDocumentProvider.metadata));
context.subscriptions.push(commands.registerCommand(Commands.LEARN_MORE_ABOUT_REFACTORING, async (kind: CodeActionKind) => {
const sectionId: string = javaRefactorKinds.get(kind) || '';
markdownPreviewProvider.show(context.asAbsolutePath(path.join('document', `${Commands.LEARN_MORE_ABOUT_REFACTORING}.md`)), 'Java Refactoring', sectionId, context);
}));
Comment on lines +302 to +307
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we check the new API is available (think Theia/Che) before registering the new provider?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there any guidance that we could follow to do such checks?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume something along the lines of if (CodeActionProviderMetadata !== null). But I'm no typescript expert

Copy link
Collaborator Author

@jdneo jdneo Jul 20, 2020

Choose a reason for hiding this comment

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

CodeActionProviderMetadata is only used as a prarm when register the code action provider, so probably it's not able to be used for API checking. Using env.appName.includes('VS Code')?

// BTW, will theia/che honor the engines field in package.json?

Copy link
Collaborator

Choose a reason for hiding this comment

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

env.appName.includes('VS Code') is bad, since we'd need to rerelease vscode-java once Theia/Che support the feature.

// BTW, will theia/che honor the engines field in package.json?

That's a question for @benoitf

I don't understand why we can't do a check on CodeActionProviderMetadata (&& CodeActionProviderMetadata.documentation) like we do for https://github.com/redhat-developer/vscode-java/blob/master/src/semanticTokenProvider.ts#L8

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jdneo if you link to a vsix build, maybe @benoitf can test it (display the refactoring menu, see there are no errors)

Copy link
Collaborator Author

@jdneo jdneo Jul 20, 2020

Choose a reason for hiding this comment

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

Hi @benoitf,

Would you mind to help test this feature? Here is the vsix link. To test it, you can open a Java file, right click in the editor and click Learn more about Java refactorings (if it exists in the context menu).

More details can be found in the gif

Thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

refactoring

custom documentation does not appear but it's expected as not yet supported

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you @benoitf!

@fbricon Seems that a RefactorDocumentProvider is registered for nothing.

Screen Shot 2020-07-20 at 9 38 58 PM

});
}

Expand Down
1 change: 1 addition & 0 deletions test/standard-mode-suite/extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ suite('Java Language Extension - Standard', () => {
const packageJSON = JSON.parse(fs.readFileSync(path.join(__dirname, '../../../test/resources/packageExample.json'), 'utf8'));
const fakedExtension = {
id: 'test',
extensionUri: null,
extensionPath: '',
isActive: true,
packageJSON,
Expand Down
19 changes: 19 additions & 0 deletions webview-resources/document.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
.btn {
border: 0;
color: var(--vscode-button-foreground);
background-color: var(--vscode-button-background);
}

.btn svg {
fill: var(--vscode-button-foreground);
}

.btn:hover {
background-color: var(--vscode-button-hoverBackground);
}

.floating-bottom-right {
position: fixed;
bottom: 1rem;
right: 1rem;
}
Loading