Skip to content

Commit

Permalink
refactor: review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@wkillerud
wkillerud committed Sep 28, 2024
1 parent 1e1db61 commit 07bca39
Show file tree
Hide file tree
Showing 12 changed files with 108 additions and 75 deletions.
6 changes: 2 additions & 4 deletions docs/src/language-server/configure-a-client.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@ For example, while we may document `"somesass.scss.workspace.loadPaths": []` (an
{
"settings": {
"somesass": {
"scss": {
"workspace": {
"loadPaths": []
}
"workspace": {
"loadPaths": []
}
}
}
Expand Down
19 changes: 10 additions & 9 deletions docs/src/user-guide/settings.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Once you turn off the built-in language features you can configure Some Sass to

Now that you disabled the built-in language features you need to turn on those language features in Some Sass.

Open your user settings JSON and paste this configuration. You may want to restart VS Code to make sure the changes apply.
Open your user settings JSON and paste this configuration. Restart VS Code to make sure the changes apply.

```json
{
Expand Down Expand Up @@ -82,6 +82,15 @@ Open your user settings JSON and paste this configuration. You may want to resta

You can configure similar settings for both SCSS, Sass (indented) and CSS. There are also some settings that apply to the workspace regardless of syntax.

### Workspace

| Id | Description | Default |
| ------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------- |
| `somesass.workspace.loadPaths` | A list of paths relative to the workspace root where the language server should look for stylesheets loaded by `@use` and `@import`. `node_modules` is always included. Note that you will have to [configure your Sass compiler separately](https://sass-lang.com/documentation/cli/dart-sass/#load-path). | `[]` |
| `somesass.workspace.exclude` | List of glob patterns for directories that are excluded in the initial scan for Sass files. Files in the exclude list will still be processed if referenced by `@use`, `@forward` and `@import` (for example a depencendy you use from `node_modules`). | `["**/.git/**", "**/node_modules/**"]` |
| `somesass.workspace.logLevel` | Control how much gets logged to the Output window. Possible values are `"silent"`, `"fatal"`, `"error"`, `"warn"`, `"info"`, `"debug"` and `"trace"`. | `"info"` |
| `some-sass.trace.server` | Log the messages sent between VS Code and the Some Sass language server. Possible values are `"off"`, `"messages"` and `"verbose"` | `"off"` |

### SCSS

For brevity the ID column omits the `somesass.scss` prefix. For example, to use the setting `codeAction.enabled` use the ID `somesass.scss.codeAction.enabled`.
Expand Down Expand Up @@ -171,11 +180,3 @@ For brevity the ID column omits the `somesass.css` prefix. For example, to use t
| `signatureHelp.enabled` | Enable or disable signature help. | `false` |
| `workspaceSymbol.enabled` | Enable or disable workspace symbol. | `false` |
| `customData` | A list of relative file paths pointing to JSON files following the [custom data format](https://github.com/microsoft/vscode-css-languageservice/blob/master/docs/customData.md). Some Sass loads custom data on startup to enhance its CSS support for CSS custom properties (variables), at-rules, pseudo-classes, and pseudo-elements you specify in the JSON files. The file paths are relative to workspace and only workspace folder settings are considered. | `[]` |

### Workspace

| Id | Description | Default |
| ------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------- |
| `somesass.workspace.loadPaths` | A list of paths relative to the workspace root where the language server should look for stylesheets loaded by `@use` and `@import`. `node_modules` is always included. Note that you will have to [configure your Sass compiler separately](https://sass-lang.com/documentation/cli/dart-sass/#load-path). | `[]` |
| `somesass.workspace.exclude` | List of glob patterns for directories that are excluded in the initial scan for Sass files. Files in the exclude list will still be processed if referenced by `@use`, `@forward` and `@import` (for example a depencendy you use from `node_modules`). | `["**/.git/**", "**/node_modules/**"]` |
| `somesass.workspace.logLevel` | Control how much gets logged to the Output window. | `"info"` |
6 changes: 3 additions & 3 deletions packages/language-server/src/configuration.ts
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import {
defaultConfiguration,
type LanguageServiceConfiguration,
type LanguageServerConfiguration,
} from "@somesass/language-services";
import { Logger } from "./logger";

export function toNewConfiguration(
v1: Partial<ConfigurationV1>,
log: Logger,
): LanguageServiceConfiguration {
): LanguageServerConfiguration {
const newSettings = Object.assign({}, defaultConfiguration, v1);
if (v1.loadPaths) {
log.info("somesass.loadPaths is now somesass.workspace.loadPaths");
Expand Down Expand Up @@ -73,7 +73,7 @@ export function toNewConfiguration(
}

export function isOldConfiguration(
maybeV1: Partial<LanguageServiceConfiguration | ConfigurationV1>,
maybeV1: Partial<LanguageServerConfiguration | ConfigurationV1>,
) {
const asV1 = maybeV1 as Partial<ConfigurationV1>;
if (typeof asV1.loadPaths !== "undefined") return true;
Expand Down
6 changes: 5 additions & 1 deletion packages/language-server/src/embedded.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ type Region = {
};

export function isFileWhereScssCanBeEmbedded(path: string) {
if (path.endsWith(".scss") || path.endsWith(".sass")) {
if (
path.endsWith(".scss") ||
path.endsWith(".sass") ||
path.endsWith(".css")
) {
return false;
}
return true;
Expand Down
97 changes: 64 additions & 33 deletions packages/language-server/src/server.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import {
defaultConfiguration,
getLanguageService,
LanguageService,
LanguageServiceConfiguration,
LanguageServerConfiguration,
LanguageConfiguration,
EditorConfiguration,
} from "@somesass/language-services";
import {
ClientCapabilities,
Expand Down Expand Up @@ -31,17 +33,13 @@ import {
import { getSassRegionsDocument } from "./embedded";
import WorkspaceScanner from "./workspace-scanner";
import { createLogger, type Logger } from "./logger";
import {
EditorConfiguration,
LanguageConfiguration,
} from "@somesass/language-services/dist/language-services-types";
import merge from "lodash.merge";

export class SomeSassServer {
private readonly connection: Connection;
private readonly runtime: RuntimeEnvironment;
private readonly log: Logger;
private configuration: LanguageServiceConfiguration = defaultConfiguration;
private configuration: LanguageServerConfiguration = defaultConfiguration;

constructor(connection: Connection, runtime: RuntimeEnvironment) {
this.connection = connection;
Expand Down Expand Up @@ -72,7 +70,6 @@ export class SomeSassServer {
logger: this.log,
});

// TODO: migrate to workspace folders
workspaceRoot = URI.parse(params.rootUri!);
this.log.info(`Workspace root ${workspaceRoot}`);

Expand Down Expand Up @@ -131,9 +128,9 @@ export class SomeSassServer {
});

const applyConfiguration = (
somesass: Partial<ConfigurationV1 | LanguageServiceConfiguration>,
somesass: Partial<ConfigurationV1 | LanguageServerConfiguration>,
editor: Partial<EditorConfiguration>,
): LanguageServiceConfiguration => {
): LanguageServerConfiguration => {
if (isOldConfiguration(somesass)) {
this.log.warn(
`Your somesass configuration uses old setting names. They will continue to work for some time, but it's recommended you change your settings to the new names. For all the available settings see https://wkillerud.github.io/some-sass/user-guide/settings.html`,
Expand All @@ -149,7 +146,7 @@ export class SomeSassServer {
);
}

const settings: LanguageServiceConfiguration = merge(
const settings: LanguageServerConfiguration = merge(
defaultConfiguration,
somesass,
{
Expand All @@ -158,17 +155,15 @@ export class SomeSassServer {
},
},
);
settings.workspace.workspaceRoot = workspaceRoot;

if (settings.workspace.logLevel) {
this.log.setLogLevel(settings.workspace.logLevel);
}
settings.workspace.workspaceRoot = workspaceRoot;

this.configuration = settings;
if (ls) {
ls.configure(settings);
}

this.log.setLogLevel(settings.workspace.logLevel);
this.log.debug("Applied user configuration");
this.log.trace(JSON.stringify(this.configuration, null, 2));

Expand Down Expand Up @@ -199,7 +194,7 @@ export class SomeSassServer {
}

let [somesass, editor] = configs as [
Partial<LanguageServiceConfiguration | ConfigurationV1>,
Partial<LanguageServerConfiguration | ConfigurationV1>,
Partial<EditorConfiguration>,
];

Expand Down Expand Up @@ -276,7 +271,9 @@ export class SomeSassServer {
}
await doDiagnostics(params);
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
}
});

Expand All @@ -286,7 +283,9 @@ export class SomeSassServer {
ls.onDocumentChanged(params.document);
await doDiagnostics(params);
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
}
});

Expand Down Expand Up @@ -314,7 +313,9 @@ export class SomeSassServer {

await workspaceScanner.scan(newFiles);
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
}
});

Expand All @@ -338,7 +339,9 @@ export class SomeSassServer {
return null;
}
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand All @@ -360,7 +363,9 @@ export class SomeSassServer {
return null;
}
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand All @@ -382,7 +387,9 @@ export class SomeSassServer {
return null;
}
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand All @@ -404,7 +411,9 @@ export class SomeSassServer {
return null;
}
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand All @@ -429,7 +438,9 @@ export class SomeSassServer {
return null;
}
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand All @@ -453,7 +464,9 @@ export class SomeSassServer {
return null;
}
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand All @@ -479,7 +492,9 @@ export class SomeSassServer {
return null;
}
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand All @@ -490,7 +505,9 @@ export class SomeSassServer {
const result = ls.findWorkspaceSymbols(params.query);
return result;
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand Down Expand Up @@ -543,7 +560,9 @@ export class SomeSassServer {

return result;
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand All @@ -568,7 +587,9 @@ export class SomeSassServer {
return null;
}
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand All @@ -594,7 +615,9 @@ export class SomeSassServer {
return null;
}
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand All @@ -618,7 +641,9 @@ export class SomeSassServer {
return null;
}
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand All @@ -643,7 +668,9 @@ export class SomeSassServer {
return null;
}
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand All @@ -664,7 +691,9 @@ export class SomeSassServer {
return null;
}
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand All @@ -688,7 +717,9 @@ export class SomeSassServer {
return null;
}
} catch (e) {
this.log.debug((e as Error).message);
const error = e as Error;
this.log.debug(String(error));
if (error.stack) this.log.debug(error.stack);
return null;
}
});
Expand Down
Loading

0 comments on commit 07bca39

Please sign in to comment.