search

mjmlio / vscode-mjml

MJML preview, lint, compile for Visual Studio Code.
https://marketplace.visualstudio.com/items?itemName=attilabuti.vscode-mjml
MIT License
178 stars 24 forks source link

Command 'MJML: Screenshot' resulted in an error (command 'mjml.screenshot' not found) #9

Closed lcherone closed 4 years ago

lcherone commented 4 years ago

When attempting to take a screenshot of the file via Command Pallet (Ctrl-Shift-P), the above error pops up. No biggie as can take manual screen but looks like a bug.

Peek 2020-11-25 16-51

Specifications

VS Code version: 1.51.1
Commit: e5a624b788d92b8d34d1392e4c4d9789406efe8f
Date: 2020-11-10T23:31:29.624Z
Electron: 9.3.3
Chrome: 83.0.4103.122
Node.js: 12.14.1
V8: 8.3.110.13-electron.0
OS: Linux x64 4.15.0-123-generic

Installed via extensions UI bog-standard way, little popup said it was updating mjml and completed without issue, everything else seems to work.

lcherone commented 4 years ago

My bad,

I looked through the code and can see that the message which says MJML needs to be rebuilt for your current platform. Please wait for the installation to finish... is actually just installing phantom-js and once its done the following shows JML's been updated. Please restart VSCode in order to continue using MJML. - I/one would presume from them messages that its an update and is unrelated to screenshots when it all works (except screens) afterwards.

I restarted vs-code and it works fine after some initial boot time the screen size prompt pops up.

My suggestion would be to simply tell the user your installing phantom-js vs saying doing an update, then specifically tell them to restart to enable screenshot functionality, though as long as one actually reads the message and restarts It should be fine.

Though you could also probably make a method for the code in the constructor 

subscriptions.push(
  commands.registerCommand('mjml.screenshot', () => {
     this.renderMJML(false)
  }),

  commands.registerCommand('mjml.multipleScreenshots', () => {
     this.renderMJML(true)
  }),
)

// passing subscriptions object to the rebuild method call

... and then call it on the successful install of phantom-js line 63, then it won't need the restart message, it would just work.

Did you know phantom-js is deprecated?

iRyusa commented 4 years ago

It's a fork of first "community" extension here so we've never really looked that part of the extension. PhantomJS is just archived not really deprecated, i'm not sure we really want to re-do that part just for this ?

lcherone commented 4 years ago

np, sorry I just saw on npm that it says its deprecated https://www.npmjs.com/package/phantomjs-prebuilt