Skip to content
This repository has been archived by the owner on Nov 7, 2022. It is now read-only.

New file command #73

Merged
merged 2 commits into from
Feb 11, 2020
Merged

New file command #73

merged 2 commits into from
Feb 11, 2020

Conversation

ckipp01
Copy link
Member

@ckipp01 ckipp01 commented Feb 10, 2020

So after reading scalameta/metals-vscode#183 I'm assuming this will change a bit, but I figured I'd create this pr just to show the initial idea I had about how to implement this. It seems to be working pretty well. 😄

I'm excited to start being able to use this feature. I've included a couple points/questions I'd love your input on @gabro.

closes #34

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
Copy link
Member

@gabro gabro left a comment

Choose a reason for hiding this comment

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

Some suggestions here and there, but overall LGTM 👍

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/metalsProtocol.ts Outdated Show resolved Hide resolved
@ckipp01 ckipp01 marked this pull request as ready for review February 11, 2020 18:36
@ckipp01 ckipp01 requested a review from gabro February 11, 2020 18:36
@ckipp01
Copy link
Member Author

ckipp01 commented Feb 11, 2020

As always, thanks for the great review @gabro. I addressed the points and cleaned it up.

Copy link
Member

@gabro gabro left a comment

Choose a reason for hiding this comment

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

Just a tiny detail, the it’s 👍

- don't mix async/await and then/catch
- add in some types
- simplify the way we target file type selection
@ckipp01
Copy link
Member Author

ckipp01 commented Feb 11, 2020

Fixed the last thing. 👍

@ckipp01 ckipp01 merged commit f9876b8 into scalameta:master Feb 11, 2020
@ckipp01 ckipp01 deleted the new-file branch February 11, 2020 19:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a quick command to add a worksheet
2 participants