Skip to content

prompt user to explicitly allow formatting using the built in formatter #558

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

Merged
merged 3 commits into from
Aug 30, 2022
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
8 changes: 8 additions & 0 deletions client/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,14 @@ export function activate(context: ExtensionContext) {
// language client, and because of that requires a full restart.
context.subscriptions.push(
workspace.onDidChangeConfiguration(({ affectsConfiguration }) => {
// Send a general message that configuration has updated. Clients
// interested can then pull the new configuration as they see fit.
client
.sendNotification("workspace/didChangeConfiguration")
.catch((err) => {
window.showErrorMessage(String(err));
});

// Put any configuration that, when changed, requires a full restart of
// the server here. That will typically be any configuration that affects
// the capabilities declared by the server, since those cannot be updated
Expand Down
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,12 @@
"type": "object",
"title": "ReScript",
"properties": {
"rescript.settings.allowBuiltInFormatter": {
"scope": "language-overridable",
"type": "boolean",
"default": false,
"description": "Whether you want to allow the extension to format your code using its built in formatter when it cannot find a ReScript compiler version in your current project to use for formatting."
},
"rescript.settings.askToStartBuild": {
"scope": "language-overridable",
"type": "boolean",
Expand Down
31 changes: 30 additions & 1 deletion server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { WorkspaceEdit } from "vscode-languageserver";
import { filesDiagnostics } from "./utils";

interface extensionConfiguration {
allowBuiltInFormatter: boolean;
askToStartBuild: boolean;
inlayHints: {
enable: boolean;
Expand All @@ -37,6 +38,7 @@ interface extensionConfiguration {
// All values here are temporary, and will be overridden as the server is
// initialized, and the current config is received from the client.
let extensionConfiguration: extensionConfiguration = {
allowBuiltInFormatter: false,
askToStartBuild: true,
inlayHints: {
enable: false,
Expand All @@ -45,6 +47,8 @@ let extensionConfiguration: extensionConfiguration = {
codeLens: false,
binaryPath: null,
};
// Below here is some state that's not important exactly how long it lives.
let hasPromptedAboutBuiltInFormatter = false;
let pullConfigurationPeriodically: NodeJS.Timeout | null = null;

// https://microsoft.github.io/language-server-protocol/specification#initialize
Expand Down Expand Up @@ -736,7 +740,13 @@ function format(msg: p.RequestMessage): Array<p.Message> {
let projectRootPath = utils.findProjectRootOfFile(filePath);
let bscBinaryPath =
projectRootPath === null ? null : findBscBinary(projectRootPath);
let formattedResult = utils.formatCode(bscBinaryPath, filePath, code);

let formattedResult = utils.formatCode(
bscBinaryPath,
filePath,
code,
extensionConfiguration.allowBuiltInFormatter
);
if (formattedResult.kind === "success") {
let max = code.length;
let result: p.TextEdit[] = [
Expand All @@ -754,6 +764,25 @@ function format(msg: p.RequestMessage): Array<p.Message> {
result: result,
};
return [response];
} else if (formattedResult.kind === "blocked-using-built-in-formatter") {
// Let's only prompt the user once about this, or things might become annoying.
if (hasPromptedAboutBuiltInFormatter) {
return [fakeSuccessResponse];
}
hasPromptedAboutBuiltInFormatter = true;

let params: p.ShowMessageParams = {
type: p.MessageType.Warning,
message: `Formatting not applied! Could not find the ReScript compiler in the current project, and you haven't configured the extension to allow formatting using the built in formatter. To allow formatting files not strictly part of a ReScript project using the built in formatter, [please configure the extension to allow that.](command:workbench.action.openSettings?${encodeURIComponent(
JSON.stringify(["rescript.settings.allowBuiltInFormatter"])
)})`,
};
let response: p.NotificationMessage = {
jsonrpc: c.jsonrpcVersion,
method: "window/showMessage",
params: params,
};
return [fakeSuccessResponse, response];
} else {
// let the diagnostics logic display the updated syntax errors,
// from the build.
Expand Down
17 changes: 15 additions & 2 deletions server/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,18 @@ type execResult =
error: string;
};

type formatCodeResult =
| execResult
| {
kind: "blocked-using-built-in-formatter";
};

export let formatCode = (
bscPath: p.DocumentUri | null,
filePath: string,
code: string
): execResult => {
code: string,
allowBuiltInFormatter: boolean
): formatCodeResult => {
let extension = path.extname(filePath);
let formatTempFileFullPath = createFileInTempDir(extension);
fs.writeFileSync(formatTempFileFullPath, code, {
Expand All @@ -100,6 +107,12 @@ export let formatCode = (
result: result.toString(),
};
} else {
if (!allowBuiltInFormatter) {
return {
kind: "blocked-using-built-in-formatter",
};
}

let result = runAnalysisAfterSanityCheck(
formatTempFileFullPath,
["format", formatTempFileFullPath],
Expand Down