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 template graph preview #106

Merged
merged 26 commits into from
Jun 12, 2020

Conversation

miparnisari
Copy link
Contributor

@miparnisari miparnisari commented May 26, 2020

aws-cloudformation/cfn-lint#1411

Description of changes:
image

image

Try it out by pulling this PR, running npm install && npm run compile, open this PR in VS Code and then launching the extension. (Ensure you have run pip3 install cfn-lint pydot --upgrade too)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

client/src/extension.ts Outdated Show resolved Hide resolved
client/src/extension.ts Outdated Show resolved Hide resolved
server/src/server.ts Outdated Show resolved Hide resolved
client/src/extension.ts Outdated Show resolved Hide resolved
@PatMyron
Copy link
Contributor

PatMyron commented May 29, 2020

Launch extension seems to work for me:

git clone https://github.com/aws-cloudformation/aws-cfn-lint-visual-studio-code
gh pr checkout 106
cd aws-cfn-lint-visual-studio-code/
npm install
npm run compile
code . 

run button

also works when building and installing .vsix:

npm install -g node@latest
npm install -g npm@latest
npm install -g vscode@latest
npm install -g vsce@latest
npm install
npm run compile
vsce package

Screen Shot 2020-05-29 at 1 51 35 PM

@PatMyron
Copy link
Contributor

@miparnisari not sure how this is working currently, but it'd be nice to eventually allow the resource dependency graph preview to occupy the entire side panel. It seems to currently be limited height-wise at least:

graph-preview

Comment on lines 162 to 164
<script src="https://unpkg.com/d3@5.16.0/dist/d3.min.js"></script>
<script src="https://unpkg.com/@hpcc-js/wasm@0.3.11/dist/index.min.js"></script>
<script src="https://unpkg.com/d3-graphviz@3.0.5/build/d3-graphviz.min.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

Like the move off of cdnjs since it's curated and missing libraries
We should add Subresource Integrity:

Suggested change
<script src="https://unpkg.com/d3@5.16.0/dist/d3.min.js"></script>
<script src="https://unpkg.com/@hpcc-js/wasm@0.3.11/dist/index.min.js"></script>
<script src="https://unpkg.com/d3-graphviz@3.0.5/build/d3-graphviz.min.js"></script>
<script src="https://cdn.jsdelivr.net/npm/d3@5.16.0/dist/d3.min.js" integrity="sha256-Xb6SSzhH3wEPC4Vy3W70Lqh9Y3Du/3KxPqI2JHQSpTw=" crossorigin="anonymous"></script>
<script src="https://cdn.jsdelivr.net/npm/@hpcc-js/wasm@0.3.11/dist/index.min.js" integrity="sha256-ed1KzxE+MS5A4K5j2j73fLv/1MNRWJ73e8N0V/mU/hU=" crossorigin="anonymous"></script>
<script src="https://cdn.jsdelivr.net/npm/d3-graphviz@3.0.5/build/d3-graphviz.min.js" integrity="sha256-dZ4Que6P88a308S9XMJrqbcD7iEjcaejEPLGkEJrEZQ=" crossorigin="anonymous"></script>

https://www.jsdelivr.com/package/npm/d3?version=5.16.0
https://www.jsdelivr.com/package/npm/@hpcc-js/wasm?version=0.3.11
https://www.jsdelivr.com/package/npm/d3-graphviz?version=3.0.5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. We're also supposed to add a content security policy, but I couldn't get this to work

@miparnisari
Copy link
Contributor Author

it'd be nice to eventually allow the resource dependency graph preview to occupy the entire side panel

Try again :)

@miparnisari miparnisari marked this pull request as ready for review May 30, 2020 07:09
@miparnisari miparnisari requested a review from PatMyron May 30, 2020 07:09
@PatMyron

This comment has been minimized.

@miparnisari miparnisari changed the title Add template preview as graph Add template preview Jun 5, 2020
@miparnisari
Copy link
Contributor Author

@kddejong @PatMyron if you review this i'll buy you a beer (or whatever drink you like)

PatMyron added a commit to PatMyron/cfn-lint-online that referenced this pull request Jun 12, 2020
to support upcoming CloudFormation template resource dependency graph preview:
aws-cloudformation/cfn-lint-visual-studio-code#106
@PatMyron
Copy link
Contributor

working incredibly well in dark mode, light mode and online 🎉

Screen Shot 2020-06-11 at 10 17 51 PM

Screen Shot 2020-06-11 at 10 08 21 PM

sometimes the preview button disappears when switching back to a CloudFormation template from editing a non-CloudFormation JSON/YAML file until the CloudFormation template is re-edited, re-saved, or re-opened again, but just something to keep in mind


really excited for this to launch 🥳
only thing left is a mention of pydot in the Requirements section of the README
we should also update the README screenshot eventually to a better one including this 🎉

README.md Outdated Show resolved Hide resolved
@miparnisari miparnisari merged commit 089b197 into aws-cloudformation:master Jun 12, 2020
@kddejong
Copy link
Contributor

awesome new feature!

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.

None yet

3 participants