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

Introduce line marker for wasm_bindgen attributes in WebAssembly projects #5933

Merged
merged 2 commits into from
Nov 9, 2020

Conversation

avrong
Copy link
Member

@avrong avrong commented Aug 14, 2020

Expected workflow: navigating from Rust statement to JS one. Then having the ability to navigate to JS parts related to it.

Current implementation

How it works

Related to #3066

@avrong avrong force-pushed the wasm-bindgen-interop branch 2 times, most recently from 4b8f0c0 to 9e3fdca Compare August 14, 2020 18:18
@Undin Undin added the feature label Aug 14, 2020
build.gradle.kts Show resolved Hide resolved
build.gradle.kts Outdated Show resolved Hide resolved
build.gradle.kts Outdated Show resolved Hide resolved
@avrong avrong force-pushed the wasm-bindgen-interop branch from 9e3fdca to ae6fa18 Compare August 15, 2020 21:14
Copy link
Member

@Undin Undin left a comment

Choose a reason for hiding this comment

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

Suggestion: use more lightweight gutter icon
The current one looks too heavy (in my opinion, of course)
For example, C++ similar icon
2020-08-17 11 40 15

Also, it would be great to come up with an extendable template for such icons because we can provide a similar feature for extern functions
Of course, don't insist to do it in this PR

@avrong
Copy link
Member Author

avrong commented Aug 17, 2020

There is an option to show JS file icon like PyCharm does for Flask templates. But I'm not sure if it is convenient to use a file icon here.

We can use something similar to the C++ icon, then I think we should make it distinctive (but not standing out) from others as wasm_bindgen is a third-party thing to the language itself and its IDE features.

image

A similar feature for extern functions would be cool. I'm not aware of the details but will try to find a way to keep these features in one area for the future.

@Undin
Copy link
Member

Undin commented Aug 17, 2020

@avrong personally, I like an approach with file icon since it doesn't require any additional icons, quite extensible and it's already used in other JB products. But we should use TS icon instead of JS in this PR, shouldn't we?
@ortem what do you think?

@artemmukhin
Copy link
Member

@Undin I think JS/TS icons is a clear and simple solution 👍

@avrong avrong force-pushed the wasm-bindgen-interop branch from ae6fa18 to 5e46f63 Compare August 17, 2020 14:49
@artemmukhin artemmukhin self-assigned this Aug 17, 2020
@artemmukhin artemmukhin added this to the v130 milestone Aug 17, 2020
@Undin Undin removed this from the v130 milestone Sep 2, 2020
@avrong avrong force-pushed the wasm-bindgen-interop branch from 5e46f63 to fc0c5e1 Compare October 17, 2020 00:14
@avrong avrong marked this pull request as ready for review October 17, 2020 00:37
@avrong avrong force-pushed the wasm-bindgen-interop branch from fc0c5e1 to 07ca8a7 Compare October 29, 2020 18:01
@avrong avrong force-pushed the wasm-bindgen-interop branch from 07ca8a7 to b371f01 Compare November 5, 2020 15:02
@avrong avrong requested a review from artemmukhin November 5, 2020 15:02
@avrong avrong force-pushed the wasm-bindgen-interop branch from b371f01 to 41b85fc Compare November 5, 2020 16:34
@artemmukhin artemmukhin added this to the v135 milestone Nov 6, 2020
@avrong avrong force-pushed the wasm-bindgen-interop branch from 41b85fc to 2d69ab4 Compare November 6, 2020 13:21
@avrong avrong requested a review from artemmukhin November 6, 2020 13:21
@avrong avrong force-pushed the wasm-bindgen-interop branch from 2d69ab4 to 8130bca Compare November 6, 2020 17:22
@avrong avrong requested a review from artemmukhin November 7, 2020 20:52
@artemmukhin
Copy link
Member

bors r+

Copy link
Member

@Undin Undin left a comment

Choose a reason for hiding this comment

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

What about tests?

@bors
Copy link
Contributor

bors bot commented Nov 9, 2020

Build succeeded:

@bors bors bot merged commit 9c6f12d into intellij-rust:master Nov 9, 2020
@artemmukhin
Copy link
Member

@Undin We are going to add them later, in a separate PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants