Open editor when clicking on notification

[alexanderchr]

Still not sure how to open the editor at the correct line/position. At least in atom you can do atom editor filename:line but it might break other editors.

One option would to let the user supply a templated string to exec instead - like atom #{file}:#{row}.

[Turbo87]

I like the idea of that templated string, although it might cause troubles when using exec(command, [arg1, arg2]) instead of a plain string

[alexanderchr]

Yeah - if we were to use a templated string I think we'd have to drop using the second argument of exec.

[Turbo87]

Yeah - if we were to use a templated string I think we'd have to drop using the second argument of exec.

not necessarily. we could allow editor: ['atom', '#{file}:#{row}'], use the first item in the array as the command and the others as arguments and replace the tokens in each of the arguments.

[alexanderchr]

Do you have any reason for wanting that syntax over a simple string? I believe that it'd make a simpler api.

Also checked the doc for child process. If we want to send the arguments in an array we will have to use spawn instead of exec. For our purposes they are essentially the same - the only difference is in how they return output.

[Turbo87]

@alexanderchr I am worried about escaping properly if e.g. the filepath contains spaces or any other kinds of special characters (&, ...)

[alexanderchr]

@Turbo87 True. Didn't think about that.

I've implemented the templated strings i spoke of earlier - please see the changed commits.

[Turbo87]

@alexanderchr cool, thanks! I'll try to test this tomorrow.

[Turbo87]

@alexanderchr I've just tested this and in my current dependency setup the location info is not available at error.loc but at error.lineNumber and error.column directly.

[alexanderchr]

Interesting. What loaders are you using?

[Turbo87]

@alexanderchr I just used the project in the example folder of this repo

[alexanderchr]

Strange, I get { [SyntaxError: Unexpected token (1:14)] pos: 14, loc: Position { line: 1, column: 14 }, raisedAt: 25 } when running webpack in the example folder.

I'm running node v5.6.0, what are you using?

[Turbo87]

@alexanderchr node v5.8.0

it's been a while since I ran npm install in that project though so I might have some older dependencies in there. if that is the issue then some dependency apparently didn't care much about semver though.

[alexanderchr]

I upgraded to 5.8.0, still getting my errors in loc. Did npm install change anything for you?

[Gvozd]

@alexanderchr It seems to work for me Within a week I will update the branch to the current master I think it will be possible to release this feature in experimental status, and collect feedback from users

[Gvozd]

@alexanderchr your code is rebased to the current master welcome to new iteration - #63 😄