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 'buf format' #70

Merged
merged 4 commits into from
Apr 5, 2022
Merged

Add 'buf format' #70

merged 4 commits into from
Apr 5, 2022

Conversation

amckinney
Copy link
Contributor

@amckinney amckinney commented Mar 30, 2022

This adds a DocumentFormattingEditProvider that adds buf format to the extension.

A lot of the code here is referenced from Go's old goFormat.ts.

src/formatter.ts Outdated

// Use spawn instead of exec to avoid maxBufferExceeded error
const p = cp.spawn(this.binaryPath, [document.fileName], { cwd });
// TODO: We need to use the CancellationToken.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See this.

rubensf added 2 commits April 5, 2022 13:45
Since buf doesn't yet support reading from stdin for formatting,
this require some hacky work around of temporary files.
Copy link
Contributor Author

@amckinney amckinney left a comment

Choose a reason for hiding this comment

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

Looks great - just a few comments, but otherwise looks nearly ready to go.

src/formatter.ts Outdated Show resolved Hide resolved
src/formatter.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@amckinney
Copy link
Contributor Author

If we add support for stdin (described here), a lot of the comments about generating a random ID for temporary directories is a lesser concern - all that will be removed as soon as we have support for stdin.

@amckinney
Copy link
Contributor Author

Looks great - as long as you've verified this all works locally (i.e. formatting on save).

I can't stamp because I'm the original author, but feel free to approve on your end (as a formality on my behalf) and we can merge from there. After this lands, we can create an issue here that says to adapt the implementation to using stdin with that feature exists.

Copy link
Member

@rubensf rubensf left a comment

Choose a reason for hiding this comment

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

self-approving, w00t!

@amckinney amckinney changed the title WIP: Add 'buf format' Add 'buf format' Apr 5, 2022
@amckinney amckinney merged commit d0e46ff into main Apr 5, 2022
@amckinney amckinney deleted the format branch April 5, 2022 18:53
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.

2 participants