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

open-in-editor #483

Merged
merged 2 commits into from
Jan 12, 2018
Merged

open-in-editor #483

merged 2 commits into from
Jan 12, 2018

Conversation

Akryum
Copy link
Member

@Akryum Akryum commented Dec 26, 2017

  • Component file tooltip
  • Open in editor on click

For this to work, user webpack config needs to be modified. Example: https://github.com/vuejs/vue-devtools/pull/483/files#diff-b4cdb7abeacb810177ac96ccfd26094a (supported editors)

This should work with Nuxt out-of-the-box (using the same /_open route).

@Akryum Akryum requested review from yyx990803 and posva December 26, 2017 14:46
@yyx990803 yyx990803 mentioned this pull request Dec 26, 2017
16 tasks
@@ -39,6 +39,7 @@
"css-loader": "^0.28.7",
"eslint": "^4.13.1",
"eslint-plugin-vue-libs": "^2.0.1",
"express-open-in-editor": "^3.1.1",
Copy link
Member

Choose a reason for hiding this comment

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

why are we using the express middleware instead of open-in-editor itself?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's to be used in the webpack-dev-server directly (that uses an express server).

Copy link
Member Author

Choose a reason for hiding this comment

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

@yyx990803
Copy link
Member

Hmm, this would not work when installed as an extension, since there will be no devServer running...

@Akryum
Copy link
Member Author

Akryum commented Dec 26, 2017

It needs some change to the user webpack config yes. We can't open the file from the extension anyway.

@LinusBorg
Copy link
Member

I don't think that's a downer, that's reasonable and can be documented.

I would include it in the webpack template asap. :)

onTitleClick () {
const file = this.target.file
if (file) {
fetch(`/_open?file=${file}`).then(() => {
Copy link
Member

Choose a reason for hiding this comment

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

So this will be making a request from the origin of the extension and only works during dev - however we should be able to initiate the request from the inspected window by doing an eval like in inspectDOM!

@michalsnik michalsnik added this to the 🎄Holiday Update milestone Jan 3, 2018
@anteriovieira
Copy link
Contributor

This should work with Nuxt out-of-the-box (using the same /_open route).

I've done some testing and the implementation with nuxt is really very simple.

npm install express-open-in-editor

// nuxt.config.js
const openInEditor = require('express-open-in-editor')

module.exports = {
  ...
  serverMiddleware: [
    {
      path: '/_open',
      handler: openInEditor({
        editor: 'code',
      })
    }
  ]
}

or if you want to use something less verbose you can install the nuxt-open-in-editor module.

npm install nuxt-open-in-editor

// nuxt.config.js
module.exports = {
  ...
  modules: [
    'nuxt-open-in-editor'
  ]
}

@Akryum
Copy link
Member Author

Akryum commented Jan 4, 2018

@atinux
Copy link
Contributor

atinux commented Jan 4, 2018

Actually it's @pi0 whom implemented it, but yeah it should work out-of-the-box with Nuxt.js directly.

@pi0
Copy link
Contributor

pi0 commented Jan 4, 2018

Basically, Nuxt should also work as our middleware is using exactly same query param (?file=)

If it's going to be a standard endpoint, I would suggest using something like /open-in-editor or /__open-in-editor prevent route conflicts. We can deliver it for 1.0.0 if you agree :)

@yyx990803
Copy link
Member

The new vue-cli will also have this built-in, so yeah let's standardize this.

/__open-in-editor sounds good to me.

pi0 pushed a commit to nuxt/youch that referenced this pull request Jan 4, 2018
pi0 pushed a commit to nuxt/nuxt that referenced this pull request Jan 4, 2018
@Akryum
Copy link
Member Author

Akryum commented Jan 4, 2018

Updated to /__open-in-editor in #486

@yyx990803 yyx990803 merged commit c7f9f59 into vuejs:master Jan 12, 2018
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.

7 participants