-
-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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 editor/ide quick edit link to file/line on each error message #1566
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions. |
Is there a more generic way of setting this up, so that it would work with any compatible editor? |
Hum, I think the only way is to specify the protocol for each editor. But
not sure about it, most of the other frameworks I was accustomed to use an
ide settings like this (a pattern).
The main issue for me is that it should be a per developer setting, not a
package.json setting.
…On Wed, Feb 15, 2017 at 3:39 PM, Dan Abramov ***@***.***> wrote:
Is there a more generic way of setting this up, so that it would work with
any compatible editor?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1566 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABwQaiJxBhkllDShUoiPaEHMafUWEw5ks5rcw4hgaJpZM4MByle>
.
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
@evaisse how about reading to .env file? usually this files do not checked out to the repo, so the developers can set TEXT_EDITOR variable there |
Yes TEXT_EDITOR should be convenient but here we do provide a URI pattern,
not an editor executable path. Maybe a TEXT_EDITOR_URI ?
…On Mon, Feb 20, 2017 at 3:48 PM, Ade Viankakrisna Fadlil < ***@***.***> wrote:
@evaisse <https://github.com/evaisse> how about reading to .env file?
usually this files do not checked out to the repo, so the developers can
set TEXT_EDITOR variable there
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1566 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABwQSvLulrjI-NIxgcXZ7o1_KN2IHELks5reaeugaJpZM4MByle>
.
|
Do you have any source for text editor uri? I'm not currently familiar with it :). Can't we just use whatever default text editor the user use for default? This feature is really great, but if the user need to know how to configure it before using it i think it doesn't inline with zero config spirit :3 reading more into it, i think its better to use executable path
and .env just set to whatever executables the user want. any reason why it needs to be text editor uri? |
Hum I think we cannot do this, in fact this feature has to be triggered
from the browser-side, using a href/link. I don't think we can trigger it
using command-line without a server-side listener.
Here are some samples,
https://github.com/dhoulb/subl
https://github.com/sanduhrs/phpstorm-url-handler
atom/atom#2037
https://github.com/WizardOfOgz/atom-handler
…On Mon, Feb 20, 2017 at 5:09 PM, Ade Viankakrisna Fadlil < ***@***.***> wrote:
Do you have any source for text editor uri? I'm not currently familiar
with it :). Can't we just use whatever default text editor the user use for
default? This feature is really great, but if the user need to know how to
configure it before using it i think it doesn't inline with zero config
spirit :3
reading more into it, i think its better to use executable path
subl media-views.js:10:5 open media-views.js with sublime on line 10
column 5
atom media-views.js:10:5 open media-views.js with atom on line 10 column 5
and .env just set to whatever executables the user want.
any reason why it needs to be text editor uri?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1566 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABwQdSMXfM3JO07K36C5LDJ1sKCH6lMks5rebq8gaJpZM4MByle>
.
|
hmm, rather relying on users to install those handler, I think we can spawn another listener from this https://github.com/facebookincubator/create-react-app/tree/master/packages/react-dev-utils that is being called by scripts/start.js |
Agreed, but how to you plan to trigger command line actions using an <a
href=""></a> link ? Does react-dev-utils provide me a rpc communication
process between the browser and listeners ?
…On Tue, Feb 21, 2017 at 2:14 PM, Ade Viankakrisna Fadlil < ***@***.***> wrote:
hmm, rather relying on users to install those handler, I think we can
spawn another listener from this https://github.com/
facebookincubator/create-react-app/tree/master/packages/react-dev-utils
that is being called by scripts/start.js
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1566 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABwQSSEtdXA3Gm18J5NKpAkNIB2VWJjks5reuNHgaJpZM4MByle>
.
|
I don't think so. I'm thinking about spawning another node server, which listen to a specific url, and get the path, numbers, and column from it. Something like this
|
Can we first try to use proces.env.TEXT_EDITOR_URI & fallback to
process.env.TEXT_EDITOR with a parallel listening server ? Currently I
found that some editor still don't support the syntax you provided in
example for command line.
So we stuck to some forms of configuration.
This don't works :
/Applications/WebStorm.app/Contents/MacOS/webstorm file.js:10:10
…On Tue, Feb 21, 2017 at 8:37 PM, Ade Viankakrisna Fadlil < ***@***.***> wrote:
I don't think so. I'm thinking about spawning another node server, which
listen to a specific url, and get the path, numbers, and column from it.
Something like this
<a target="_blank" href="http://localhost:8962?path=/home/Projects/troublefile.js&line=10&column=2"></a>
const express = require('express')
const app = express()
const exec = require('child_process').exec
const port = 3012 //should use any available port
app.get('/', (req, res) => {
exec('subl '+ [req.query.path, req.query.line, req.query.column'] , (error, stdout, stderr) => {
if (error) {
console.error(`exec error: ${error}`);
return;
}
res.send('<script>window.close();</script>')
});
})
app.listen(port, (err) => {
if (err) {
return console.log('something bad happened', err)
}
console.log(`server is listening on ${port}`)
})
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1566 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABwQTTW64JPuqXVEfYZnDSbEC0ytzZqks5rezzugaJpZM4MByle>
.
|
found a nice package for it! https://www.npmjs.com/package/open-in-editor Supported editors: sublime – Sublime Text i think we should also update the docs if this is working |
@viankakrisna
|
I think this is already implemented by #2141 right now 👍 |
Filed #2805 to discuss this. Closing this PR. Thanks for the effort! |
Hi, I would like to offer the ability to get a quick link to jump to file/line on error screen. by adding in package.json an editor command file pattern like this one :
"subl://open?url=%%f&line=%%l&column=%%c"
(how can I achieve this ? what's the better solution here package /process.env ??)