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

Support for OpenFile to Intellij #35

Closed
wants to merge 19 commits into from
Closed

Conversation

zaideygrek
Copy link
Collaborator

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Apr 28, 2022

🦋 Changeset detected

Latest commit: 75ed249

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
click-to-react-component Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

setState(State.IDLE)
},
[editor]
)

const openFileInIDE = (file) => {
if (editor === 'intellij') {
const url = `http://localhost:63342/api/file/${file}`
Copy link
Owner

Choose a reason for hiding this comment

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

TIL! I'd like to move this logic up so that lineNumber & columnNumber can be used.

https://github.com/JetBrains/intellij-community/blob/a77365debaadcf00b888a977d89512f3f0f3cf9e/platform/built-in-server/src/org/jetbrains/ide/OpenFileHttpService.kt#L52-L59

This seems related to #28, #29, or #26, too 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I confirm that in the state it works well, intellij opens the file in the right row and in the right column.
Would you prefer to use another syntax?

Copy link
Collaborator

@dartess dartess May 4, 2022

Choose a reason for hiding this comment

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

@ericclemmons JetBrain's IDEs support both formats: file.kt:100:34 and file.kt&line=100&column=34. No reason for additional logic here. It is just work.

@dartess
Copy link
Collaborator

dartess commented Apr 29, 2022

add intellij to export type Editor = 'vscode' | 'vscode-insiders' please

@dartess
Copy link
Collaborator

dartess commented Apr 29, 2022

jetbrains IDEs partially support URL handling, like this:

webstorm://open?file=/Path/To/File.tsx&line=4&column=5

The following schemes are supported:

'webstorm',
'phpstorm', // also occupies 'idea'
'pycharm',
'x-mine', // rubymine
'goland',
'idea', // may conflict with phpstorm

Unfortunately this only works for macos.

But I still think it might be faster than opening via http://localhost:63342

Perhaps it is worth doing a check on the operating system and doing two ways? What do you think?

@zaideygrek
Copy link
Collaborator Author

jetbrains IDEs partially support URL handling, like this:

webstorm://open?file=/Path/To/File.tsx&line=4&column=5

The following schemes are supported:

'webstorm',
'phpstorm', // also occupies 'idea'
'pycharm',
'x-mine', // rubymine
'goland',
'idea', // may conflict with phpstorm

Unfortunately this only works for macos.

I tried this methos but jetbrains got an error: "Unsupported protocol" regression for URI handler.
There are unresolved issues like https://youtrack.jetbrains.com/issue/IDEA-284396

The safest method remains http://localhost:63342, it is fast and simple. What do you think ?

@dunklesToast
Copy link

The safest method remains http://localhost:63342, it is fast and simple. What do you think ?

Just noticed that URL Handling works all the time, even when the IDE is closed, it'll then open it. The localhost server is only active when the IDE is open (which should be the case most of the time)

@dartess
Copy link
Collaborator

dartess commented May 4, 2022

The localhost server is only active when the IDE is open (which should be the case most of the time)

I think it makes sense to add this to the documentation.

=======

We can implement both methods and describe their differences in the documentation. A consumer will choose a desired option. There is not even a coincidence of the names of the editors.

It seems to me that this PR does its job well.

I'd like to add schema support for specific IDEs later (if no one gets ahead of me).

@zaideygrek
Copy link
Collaborator Author

I added the possibility to change the default editor in the context menu. It stores the value in localstorage, so the project team can have different IDE preferences. What do you think ?

Pasted Graphic 7

image

@zaideygrek zaideygrek requested a review from ericclemmons May 8, 2022 22:42
@ryota-murakami
Copy link

Thank you for everyone!
I'm pretty glad to this PR tractions as a WebStorm user!

@dunklesToast
Copy link

I added the possibility to change the default editor in the context menu. It stores the value in localstorage, so the project team can have different IDE preferences. What do you think ?

Not a bad idea. Could we somehow also create a more persistent solution to this? While developing I often clear the whole application data to reset something or test a fresh state. This would always reset this, right?
I only have two idea which are both not the best but maybe someone else has better ones.

  1. Environment Variable
    This could be tricky, because IIRC CRA only allows env vars prefixed with REACT_APP_ , but this would probably not work with Next.js or so out of the box (as Next.js needs to have env's specifically passed and then they are under publicRuntimeConfig and not process.env).
  2. Read from window object
    This ones probably a bit dirty / hacky but we could define a property (like window.CLICK_TO_COMPONENT_EDITOR) which users could set via UserScripts or some Custom JS Injector.

The order could be smth like "localStorage > window > environment" so localStorage would override window and env and window would take presence before environment if localStorage has nothing stored. Does that make sense to you?

@dartess
Copy link
Collaborator

dartess commented May 9, 2022

I added the possibility to change the default editor in the context menu. It stores the value in localstorage, so the project team can have different IDE preferences. What do you think ?

Great idea. But sounds like a separate PR. I'm afraid adding new features (which are yet to be discussed) will slow down the approve of this PR.

@dartess
Copy link
Collaborator

dartess commented May 9, 2022

@dunklesToast another dirty thought is to somehow use GET parameters. For example, take initial value from there and put it in LS or window.

@zaideygrek
Copy link
Collaborator Author

Great idea. But sounds like a separate PR. I'm afraid adding new features (which are yet to be discussed) will slow down the approve of this PR.

You're right ! I revert code to change the default editor in the context menu. I will make a separate PR.

@zaideygrek
Copy link
Collaborator Author

@ericclemmons I'm waiting for your instructions if you have any, thanks

@dartess
Copy link
Collaborator

dartess commented May 30, 2022

@ericclemmons hi! any updates here?

@JohnyTheCarrot
Copy link

Any update on this? Would love to have it, might just have to check out this PR if it isn't going to be merged.

@mmelikyan
Copy link

@zaideygrek Looks like this project is abandoned. We're gonna have to use a fork to have this feature implemented. :/

@ericclemmons per chance, could you please clarify if you still plan on supporting this repo? thank you!

@ericclemmons
Copy link
Owner

ericclemmons commented Oct 6, 2022 via email

@zaideygrek
Copy link
Collaborator Author

What do you think about this chrome extension locatorJs ?

@mmelikyan
Copy link

I still think this is a fantastic solution and could actually be turned into a big product with more features, but as-is it's awesome doing its job!

I've tried LocatorJs and it's not nearly as intuitive as click-to-component.

Concerning the IntelliJ support that this PR addresses, I've tested with WebStorm and works like a charm ✨

Please consider merging this :))

@ericclemmons
Copy link
Owner

I added
@dartess, @mmelikyan , & @zaideygrek collaborators.

Previously, this project was supported by Stripe but is not any longer.

@dartess
Copy link
Collaborator

dartess commented Apr 3, 2023

about the comment above: sounds good, I think I can spend a little time on it. but I don't have much experience with versioning support and stuff around releases.

@ericclemmons do you still own and release new versions? If so, can you add an available CONTRIBUTING.md to the project?

@dartess
Copy link
Collaborator

dartess commented Apr 3, 2023

about this PR: @zaideygrek will you have time to update this PR or will I pick up this task?

@zaideygrek
Copy link
Collaborator Author

zaideygrek commented Apr 4, 2023

Sorry I don't have time to clean the code and continue this PR.

@alanhe421
Copy link
Contributor

alanhe421 commented Jan 8, 2025

I made a PR

#98

But alas, there has been no merger.

@zaideygrek zaideygrek closed this Jan 8, 2025
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.

8 participants