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

Add support for Format Document action #200

Merged
merged 1 commit into from
Sep 13, 2021
Merged

Add support for Format Document action #200

merged 1 commit into from
Sep 13, 2021

Conversation

adalinesimonian
Copy link
Member

Which issue, if any, is this issue related to?

Closes #25.

Is there anything in the PR that needs further explanation?

The language server has existing functionality checking when language IDs are removed from configuration (connection.onDidChangeConfiguration). This implementation adds a check for added language IDs and sends the client a notification when IDs are added or removed using the notification methods stylelint/languageIdsAdded and stylelint/languageIdsRemoved.

When the client receives these notifications, it registers or unregisters, as appropriate, a document formatting edit provider with VS Code for the given language IDs. When the provider is asked to provide formatting edits, it sends a DocumentFormattingRequest to the language server and returns the result.

When the server receives a document formatting request, it passes the document and VS Code's formatting options to getFixes. getFixes now supports a second parameter accepting formatting options. When formatting options are provided, getFixes translates the options to their respective stylelint rules and sets these rules to the base configuration passed to buildStylelintOptions. The only formatting option without a corresponding rule is trimFinalNewlines, so it is ignored.

A new test has been added, ws-formatter-test. It runs the Format Document action on test CSS after setting indentation settings in VS Code. The test succeeds if the resulting CSS matches the expected formatted output.

@ux-engineer
Copy link

Any progress on this?

@adalinesimonian
Copy link
Member Author

@ux-engineer I tried contacting someone via email a while ago to see if there'd be anyone able to review this PR. However, I haven't heard back. I'm happy to work with the maintainers of this extension to make any necessary changes or corrections.

@ux-engineer
Copy link

Hi @shinnn @ntwb @ota-meshi, could you comment on this issue and PR with pending review?

@Stanzilla
Copy link

Stanzilla commented Sep 11, 2021

If you don't hear back, would be up for publishing and maintaining a fork? Looks like there are quite a few more pull requests waiting that could be taken in as well.

@adalinesimonian
Copy link
Member Author

adalinesimonian commented Sep 11, 2021

If you don't hear back, would be up to publishing and maintaining a fork? Looks like there are quite a few more pull requests waiting that could be taken in as well.

I definitely would be up to it, but I'd like to exhaust all attempts at getting PRs in the official repository looked at before fragmenting the project into two.

Update: I've just shot a follow-up email to another member of the project and am waiting for a response.

@jeddy3
Copy link
Member

jeddy3 commented Sep 12, 2021

@adalinesimonian @ux-engineer @Stanzilla Thanks for taking an interest in this extension! I'm one of the co-creators of stylelint and I've been meaning to familiarise myself with this extension's code and release process for a while now, but haven't found time (mainly because the core volunteers have been working on the next major release of stylelint, which is a substantial piece of work because of the move to PostCSS@8).

It's fantastic that you'd like to help with this extension, and I'd like to remove any barriers so that a fork is unnecessary. Sorry that it's taken me so long to get around to this.

With your permission, I can add you all to the @stylelint/vscode team which has maintain access to this repository, i.e. access to read/write and maintain issue and pull requests. You'll then be able to review and merge this and other pull requests yourselves.

I'll then work on sharing the access details for publishing the extension (which we have in a 1password vault). Are any of you familiar with publishing VSCode extensions? If not, I can dig into the process.

@adalinesimonian @ux-engineer @Stanzilla Would you like me to add you to the @stylelint/vscode team?

@Stanzilla
Copy link

Not me at the moment, but thank you for the offer and the response!

@adalinesimonian
Copy link
Member Author

Would you like me to add you to the @stylelint/vscode team?

I'm honoured; I'd be happy to help! I have an extension published on the marketplace for VS code, albeit a language grammar and not a true extension — but I'm somewhat familiar with the process, and luckily I have the time to be able to help out.

@jeddy3
Copy link
Member

jeddy3 commented Sep 13, 2021

@adalinesimonian Fantastic! I've added you to the @stylelint/vscode team. You should have permission to manage pull requests now. Give me a holler if that's not the case.

I've also removed the 1-review-needed branch protection so that you can merge your own pull requests. Someone may not be available to review your pull requests and I don't want that being a blocker to you merging your own work.

Looks like there are quite a few more pull requests waiting that could be taken in as well.

Along the same lines, feel free to deal with (or not) the other pull requests in a way that works best for you. I don't think anything other than dependency updates have been merged for a few months, so there's probably a few low-hanging-fruit pull requests.

(I'll drop another message into the @stylelint/vscode channel about publishing access rights.)

@adalinesimonian adalinesimonian self-assigned this Sep 13, 2021
@adalinesimonian adalinesimonian added the type: enhancement a new feature that isn't related to rules label Sep 13, 2021
@adalinesimonian adalinesimonian merged commit 39d8569 into stylelint:master Sep 13, 2021
@adalinesimonian adalinesimonian deleted the format-document branch September 13, 2021 16:37
index.js Show resolved Hide resolved
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.

Add support for stylelint as formatter
5 participants