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

d2-vscode-extension #11

Closed
wants to merge 26 commits into from

Conversation

BarryNolte
Copy link
Collaborator

@BarryNolte BarryNolte commented Dec 4, 2022

First PR creating the viewer for d2 files in VsCode.

#4 - For Reference

Fixes:
#9 and #13

First PR creating the viewer for d2 files in VsCode.
Added wrapper around displayed SVG tag to allow for zooming and fitting to screen.
@alixander
Copy link
Contributor

alixander commented Dec 5, 2022

exciting, looking forward to test driving it! (feel free to add me to reviewer when it's rdy)

@BarryNolte
Copy link
Collaborator Author

@alixander It’s as ready as it’ll ever be. Only prerequisite needed is to have d2 on your path.

@alixander alixander self-requested a review December 5, 2022 03:55
Added context menus everywhere it made sense and changed temp file deletion to be more deterministic.
@BarryNolte
Copy link
Collaborator Author

If I can make a change to the d2 executable to not launch the browser when in watch mode (e.g. --nobrowser switch), I can change the extension to connect to the localhost and display the result. Although that might be a cleaner solution, it doesn't allow for multiple input files without additional work (my kludge/solution does allow for this).

@alixander
Copy link
Contributor

alixander commented Dec 6, 2022

@BarryNolte BROWSER=0 will be in the next release (https://github.com/terrastruct/d2/blob/master/ci/release/changelogs/next.md). We'll cut it tonight. You can test it now by building from source

Copy link
Contributor

@gavin-ts gavin-ts left a comment

Choose a reason for hiding this comment

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

Thanks for setting this up! I added a bunch of notes, but some of these are larger issues to tackle so I was thinking we could merge this into the dev branch after some of the smaller changes and handle the rest in their own PRs?

Looks good with the fit to screen option:
Screen Shot 2022-12-21 at 2 50 35 PM

tsconfig.json Outdated Show resolved Hide resolved
src/refreshTimer.ts Outdated Show resolved Hide resolved
src/browserWindow.ts Outdated Show resolved Hide resolved
src/browserWindow.ts Show resolved Hide resolved
src/browserWindow.ts Outdated Show resolved Hide resolved
src/docToPreviewGenerator.ts Outdated Show resolved Hide resolved
src/docToPreviewGenerator.ts Show resolved Hide resolved
src/browserWindow.ts Show resolved Hide resolved
src/browserWindow.ts Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
BarryNolte and others added 6 commits December 21, 2022 19:14
Auto generated spew, no use for it once the project is setup and working.

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>
This is why I bought one of the first commercially available spell checkers, so I could graduate college.

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>
Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>
Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>
Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>
Setup eslint and fixed errors and warnings.
Made "fit" as the default on load
Changed update timer code to be simpler
Changed preview HTML to load from disk
@BarryNolte
Copy link
Collaborator Author

@gavin-ts All requested changes made in commit: 9650284

@BarryNolte BarryNolte mentioned this pull request Dec 24, 2022
package.json Outdated Show resolved Hide resolved
@BarryNolte BarryNolte requested review from gavin-ts and aaronjheng and removed request for alixander, gavin-ts and aaronjheng December 28, 2022 20:40
Copy link
Contributor

@gavin-ts gavin-ts left a comment

Choose a reason for hiding this comment

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

nice we will want to add a note to the PR body that this fixes #9 and #13

.vscode/launch.json Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
pages/previewPage.html Outdated Show resolved Hide resolved
pages/previewPage.html Show resolved Hide resolved
src/extension.ts Show resolved Hide resolved
BarryNolte and others added 3 commits December 28, 2022 16:57
Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>
Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>
Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>
Copy link
Contributor

@gavin-ts gavin-ts left a comment

Choose a reason for hiding this comment

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

I think it should be good to merge with these changes

Also noting for a follow up, I think we will want to change the d2 call to be async since it can cause typing in the .d2 file to lag on larger layouts

src/extension.ts Outdated Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
src/extension.ts Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Contributor

@gavin-ts gavin-ts left a comment

Choose a reason for hiding this comment

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

nice! it is very nice to have the preview window with render right there while editing .d2 files 🚀

src/extension.ts Outdated Show resolved Hide resolved
.vscode/tasks.json Show resolved Hide resolved
@alixander
Copy link
Contributor

@BarryNolte this looks great, i think this is ready to merge, just wanted to double check with you it's set?

@BarryNolte
Copy link
Collaborator Author

@BarryNolte this looks great, i think this is ready to merge, just wanted to double check with you it's set?

Yes, please.

PR #16 has more features, some bug fixes and few better ways of doing things as I figured them out. After that PR is merged, then we can drill down on bugs/features/enhancements with more targeted and smaller PR's.

@alixander
Copy link
Contributor

@BarryNolte could you sign the commits please? I have it turned on as a branch protection org-wide. terrastruct/d2#557 (comment)

BarryNolte and others added 4 commits January 11, 2023 17:55
First PR creating the viewer for d2 files in VsCode.

d2-vscode-extension

Added wrapper around displayed SVG tag to allow for zooming and fitting to screen.

Revert "d2-vscode-extension"

This reverts commit 55049ca.

Revert "Revert "d2-vscode-extension""

This reverts commit d189451.

Added context menus and code cleanup

Added context menus everywhere it made sense and changed temp file deletion to be more deterministic.

Update tsconfig.json

Auto generated spew, no use for it once the project is setup and working.

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>

Update src/refreshTimer.ts

This is why I bought one of the first commercially available spell checkers, so I could graduate college.

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>

Update src/docToPreviewGenerator.ts

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>

Update src/browserWindow.ts

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>

Update src/browserWindow.ts

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>

Changes from Review

Setup eslint and fixed errors and warnings.
Made "fit" as the default on load
Changed update timer code to be simpler
Changed preview HTML to load from disk

Updated branding

Update package.json

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>

Update package.json

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>
Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>
@BarryNolte
Copy link
Collaborator Author

@BarryNolte could you sign the commits please? I have it turned on as a branch protection org-wide. terrastruct/d2#557 (comment)

I tried, but failed to get all commits signed. Couldn't get beyond the latest commit being signed.

@alixander
Copy link
Contributor

@BarryNolte looks like it's been set up then, can you rewrite the commits? https://superuser.com/a/1123928 (except replace development)

BarryNolte and others added 6 commits January 11, 2023 19:49
First PR creating the viewer for d2 files in VsCode.

d2-vscode-extension

Added wrapper around displayed SVG tag to allow for zooming and fitting to screen.

Revert "d2-vscode-extension"

This reverts commit 55049ca.

Revert "Revert "d2-vscode-extension""

This reverts commit d189451.

Added context menus and code cleanup

Added context menus everywhere it made sense and changed temp file deletion to be more deterministic.

Update tsconfig.json

Auto generated spew, no use for it once the project is setup and working.

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>

Update src/refreshTimer.ts

This is why I bought one of the first commercially available spell checkers, so I could graduate college.

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>

Update src/docToPreviewGenerator.ts

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>

Update src/browserWindow.ts

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>

Update src/browserWindow.ts

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>

Changes from Review

Setup eslint and fixed errors and warnings.
Made "fit" as the default on load
Changed update timer code to be simpler
Changed preview HTML to load from disk

Updated branding

Update package.json

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>

Update package.json

Co-authored-by: gavin-ts <85081687+gavin-ts@users.noreply.github.com>
Added wrapper around displayed SVG tag to allow for zooming and fitting to screen.
Added context menus everywhere it made sense and changed temp file deletion to be more deterministic.
Setup eslint and fixed errors and warnings.
Made "fit" as the default on load
Changed update timer code to be simpler
Changed preview HTML to load from disk
@BarryNolte
Copy link
Collaborator Author

@alixander - I was able to partially get commits verified, but not all of them. I had found that article you suggested and thought it would be unpleasant to use that technique, and it was.

Suggestion: Create another PR with one signed commit that's the same as the last commit on this PR. Merge it, and abandon this one. I'll do the same for my other PR and then continue to sign each commit to avoid this in the future.

@alixander
Copy link
Contributor

yeah that'd work! @BarryNolte

@BarryNolte
Copy link
Collaborator Author

Wow, that should have been easier... See PR #19

After that's merged, we can pull the plug on this PR.

@alixander
Copy link
Contributor

sorry about the hoops @BarryNolte , ty for bearing with that, really excited to be merging all this for users (looks like the extension has been downloaded over 3k times now)

@BarryNolte
Copy link
Collaborator Author

I've had worse hoops 😄 I probably should have been signing my commits years ago. Lesson learned and my love/hate relationship with git continues. On to the next PR...

@BarryNolte BarryNolte closed this Jan 12, 2023
@BarryNolte BarryNolte deleted the d2-vscode-extension branch January 13, 2023 01:06
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.

4 participants