Added macro for implementing conversion traits on structs that wrap three-rs types. - #40

Merged
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master
Jun 24, 2017
Merged

Added macro for implementing conversion traits on structs that wrap three-rs types.#40
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master

Conversation

@sunjay

@sunjaysunjay commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.

The macro is nice and flexible and the field name is optional. I chose the syntax MyStruct#field because it seemed more natural to me than MyStruct.field or MyStruct::field (since x::y is usually for modules or methods, not fields).

See the effect of this change here

Really wanted to replace macros in src/scene.rs with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the node field, structs external to the crate do not. This causes problems in as_ref and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.

…hree-rs types.
Really wanted to replace macros in `src/scene.rs` with some macros that
could be used outside of the crate, but unfortunately that didn't workout
because while internal three objects have access to the `node` field,
structs external to the crate do not. This causes problems in `as_ref` and
if that one was going to deviate, we might as well make a single macro for
the special purpose of classes that wrap three-rs objects.
@sunjay

Copy link
Copy Markdown
ContributorAuthor

In case you're curious, my original attempt which would have been able to replace the macros in src/scene.rs is below.

The problem is in AsRef. Just &self.$field causes problems outside of this crate. Specifically, for my project, I get the error:

error[E0308]: mismatched types
--> src/player.rs:11:1
|
11 | three_as_ref_node_pointer!(Player#group);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `froggy::Pointer`, found struct `three::Group`
|
= note: expected type `&froggy::Pointer<three::Node>`
found type `&three::Group`
= note: this error originates in a macro outside of the current crate

In the macro I implemented for this PR, I had to use &self.$field.as_ref() in order for it to work outside of this crate.

/// Implement `AsRef<NodePointer>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements AsRef<NodePointer> for MyStruct using field `internal_three_type`/// three_as_ref_node_pointer!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `node`/// three_as_ref_node_pointer!(MyStruct); // equivalent to three_as_ref_node_pointer!(MyStruct#node);/// ```#[macro_export]macro_rules! three_as_ref_node_pointer {($($name:ident),*) => {
three_as_ref_node_pointer!($($name#node),*);};($($name:ident # $field:ident),*) => {
$(implAsRef<$crate::NodePointer> for $name {fn as_ref(&self) -> &$crate::NodePointer{&self.$field
}})*};}/// Implement `Deref<Target=Object>` and `DerefMut<Object>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements Deref<Target=Object> and DerefMut<Object> for MyStruct using field/// // `internal_three_type`/// three_deref_object!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `object`/// three_deref_object!(MyStruct); // equivalent to three_deref_object!(MyStruct#object);/// ```#[macro_export]macro_rules! three_deref_object {($($name:ident),*) => {
three_deref_object!($($name#object),*);};($($name:ident # $field:ident),*) => {
$(impl::std::ops::Dereffor $name {typeTarget = Object;fn deref(&self) -> &Object{&self.$field
}}impl::std::ops::DerefMutfor $name {fn deref_mut(&mutself) -> &mutObject{&mutself.$field
}})*};}

@sunjay

Copy link
Copy Markdown
ContributorAuthor

Looks great in the docs!

screenshot from 2017-06-24 00-59-27

Detailed docs:

screenshot from 2017-06-24 00-59-15

Comment threadsrc/macros.rs Outdated
/// * `Deref<Target=Object>`
/// * `DerefMut<Object>`
///
/// ```

@vitvakatuvitvakatuJun 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Replace this line with

```rust,no_run

to prevent running this as doc test (causes error during Travis CI)

@kvarkkvark left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a very well made PR, thank you!
You included extensive info on the reasoning, great comments, structure, and even provided the doc screenshots, very nice!

Comment threadsrc/macros.rs
/// three_object_wrapper!(MyStruct); // equivalent to three_object_wrapper!(MyStruct#object);
/// ```
#[macro_export]
macro_rules! three_object_wrapper {

@kvarkkvarkJun 24, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm unsure about including "three_" as a macro prefix. Do you see it as an established convention anywhere?

Edit: nevermind, it is the convention, all good!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I don't think it is. The reason I added it is because it's nice to know where macros come from. If you look at the code where I used this (linked in the PR), it's very clear that the macro being used is for this library and its types.

I'm open to another name too if you have something in mind. :)

@kvark

Copy link
Copy Markdown
Collaborator

Another thing that bothers me a bit is the use of #: while it looks ok in the code, it's not near any existing convention (as you mentioned, . or ::). I think ::, while used for methods, is really straightforward here, given that C++ has it for fields as well.

@sunjay

Copy link
Copy Markdown
ContributorAuthor

@vitvakatu@kvark I changed # to :: and added ignore to the doctest since getting it to compile with no_run was quite a bit of extra work. Adding a doctest for this would be great future work!

Let me know if you have any comments. If not, this should be good to go!

@kvark

kvark commented Jun 24, 2017 via email

Copy link
Copy Markdown
Collaborator

borsBot added a commit that referenced this pull request Jun 24, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
@bors

borsBot commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

Build succeeded

@bors
borsBot merged commit c94a970 into three-rs:masterJun 24, 2017
kvark pushed a commit that referenced this pull request Nov 10, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
Sign up for freeto 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.

3 participants

@sunjay@kvark@vitvakatu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Added macro for implementing conversion traits on structs that wrap three-rs types. - #40

Merged
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master
Jun 24, 2017
Merged

Added macro for implementing conversion traits on structs that wrap three-rs types.#40
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master

Conversation

@sunjay

@sunjaysunjay commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.

The macro is nice and flexible and the field name is optional. I chose the syntax MyStruct#field because it seemed more natural to me than MyStruct.field or MyStruct::field (since x::y is usually for modules or methods, not fields).

See the effect of this change here

Really wanted to replace macros in src/scene.rs with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the node field, structs external to the crate do not. This causes problems in as_ref and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.

…hree-rs types.
Really wanted to replace macros in `src/scene.rs` with some macros that
could be used outside of the crate, but unfortunately that didn't workout
because while internal three objects have access to the `node` field,
structs external to the crate do not. This causes problems in `as_ref` and
if that one was going to deviate, we might as well make a single macro for
the special purpose of classes that wrap three-rs objects.
@sunjay

Copy link
Copy Markdown
ContributorAuthor

In case you're curious, my original attempt which would have been able to replace the macros in src/scene.rs is below.

The problem is in AsRef. Just &self.$field causes problems outside of this crate. Specifically, for my project, I get the error:

error[E0308]: mismatched types
--> src/player.rs:11:1
|
11 | three_as_ref_node_pointer!(Player#group);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `froggy::Pointer`, found struct `three::Group`
|
= note: expected type `&froggy::Pointer<three::Node>`
found type `&three::Group`
= note: this error originates in a macro outside of the current crate

In the macro I implemented for this PR, I had to use &self.$field.as_ref() in order for it to work outside of this crate.

/// Implement `AsRef<NodePointer>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements AsRef<NodePointer> for MyStruct using field `internal_three_type`/// three_as_ref_node_pointer!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `node`/// three_as_ref_node_pointer!(MyStruct); // equivalent to three_as_ref_node_pointer!(MyStruct#node);/// ```#[macro_export]macro_rules! three_as_ref_node_pointer {($($name:ident),*) => {
three_as_ref_node_pointer!($($name#node),*);};($($name:ident # $field:ident),*) => {
$(implAsRef<$crate::NodePointer> for $name {fn as_ref(&self) -> &$crate::NodePointer{&self.$field
}})*};}/// Implement `Deref<Target=Object>` and `DerefMut<Object>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements Deref<Target=Object> and DerefMut<Object> for MyStruct using field/// // `internal_three_type`/// three_deref_object!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `object`/// three_deref_object!(MyStruct); // equivalent to three_deref_object!(MyStruct#object);/// ```#[macro_export]macro_rules! three_deref_object {($($name:ident),*) => {
three_deref_object!($($name#object),*);};($($name:ident # $field:ident),*) => {
$(impl::std::ops::Dereffor $name {typeTarget = Object;fn deref(&self) -> &Object{&self.$field
}}impl::std::ops::DerefMutfor $name {fn deref_mut(&mutself) -> &mutObject{&mutself.$field
}})*};}

@sunjay

Copy link
Copy Markdown
ContributorAuthor

Looks great in the docs!

screenshot from 2017-06-24 00-59-27

Detailed docs:

screenshot from 2017-06-24 00-59-15

Comment threadsrc/macros.rs Outdated
/// * `Deref<Target=Object>`
/// * `DerefMut<Object>`
///
/// ```

@vitvakatuvitvakatuJun 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Replace this line with

```rust,no_run

to prevent running this as doc test (causes error during Travis CI)

@kvarkkvark left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a very well made PR, thank you!
You included extensive info on the reasoning, great comments, structure, and even provided the doc screenshots, very nice!

Comment threadsrc/macros.rs
/// three_object_wrapper!(MyStruct); // equivalent to three_object_wrapper!(MyStruct#object);
/// ```
#[macro_export]
macro_rules! three_object_wrapper {

@kvarkkvarkJun 24, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm unsure about including "three_" as a macro prefix. Do you see it as an established convention anywhere?

Edit: nevermind, it is the convention, all good!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I don't think it is. The reason I added it is because it's nice to know where macros come from. If you look at the code where I used this (linked in the PR), it's very clear that the macro being used is for this library and its types.

I'm open to another name too if you have something in mind. :)

@kvark

Copy link
Copy Markdown
Collaborator

Another thing that bothers me a bit is the use of #: while it looks ok in the code, it's not near any existing convention (as you mentioned, . or ::). I think ::, while used for methods, is really straightforward here, given that C++ has it for fields as well.

@sunjay

Copy link
Copy Markdown
ContributorAuthor

@vitvakatu@kvark I changed # to :: and added ignore to the doctest since getting it to compile with no_run was quite a bit of extra work. Adding a doctest for this would be great future work!

Let me know if you have any comments. If not, this should be good to go!

@kvark

kvark commented Jun 24, 2017 via email

Copy link
Copy Markdown
Collaborator

borsBot added a commit that referenced this pull request Jun 24, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
@bors

borsBot commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

Build succeeded

@bors
borsBot merged commit c94a970 into three-rs:masterJun 24, 2017
kvark pushed a commit that referenced this pull request Nov 10, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
Sign up for freeto 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.

3 participants

@sunjay@kvark@vitvakatu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Added macro for implementing conversion traits on structs that wrap three-rs types. - #40

Merged
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master
Jun 24, 2017
Merged

Added macro for implementing conversion traits on structs that wrap three-rs types.#40
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master

Conversation

@sunjay

@sunjaysunjay commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.

The macro is nice and flexible and the field name is optional. I chose the syntax MyStruct#field because it seemed more natural to me than MyStruct.field or MyStruct::field (since x::y is usually for modules or methods, not fields).

See the effect of this change here

Really wanted to replace macros in src/scene.rs with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the node field, structs external to the crate do not. This causes problems in as_ref and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.

…hree-rs types.
Really wanted to replace macros in `src/scene.rs` with some macros that
could be used outside of the crate, but unfortunately that didn't workout
because while internal three objects have access to the `node` field,
structs external to the crate do not. This causes problems in `as_ref` and
if that one was going to deviate, we might as well make a single macro for
the special purpose of classes that wrap three-rs objects.
@sunjay

Copy link
Copy Markdown
ContributorAuthor

In case you're curious, my original attempt which would have been able to replace the macros in src/scene.rs is below.

The problem is in AsRef. Just &self.$field causes problems outside of this crate. Specifically, for my project, I get the error:

error[E0308]: mismatched types
--> src/player.rs:11:1
|
11 | three_as_ref_node_pointer!(Player#group);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `froggy::Pointer`, found struct `three::Group`
|
= note: expected type `&froggy::Pointer<three::Node>`
found type `&three::Group`
= note: this error originates in a macro outside of the current crate

In the macro I implemented for this PR, I had to use &self.$field.as_ref() in order for it to work outside of this crate.

/// Implement `AsRef<NodePointer>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements AsRef<NodePointer> for MyStruct using field `internal_three_type`/// three_as_ref_node_pointer!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `node`/// three_as_ref_node_pointer!(MyStruct); // equivalent to three_as_ref_node_pointer!(MyStruct#node);/// ```#[macro_export]macro_rules! three_as_ref_node_pointer {($($name:ident),*) => {
three_as_ref_node_pointer!($($name#node),*);};($($name:ident # $field:ident),*) => {
$(implAsRef<$crate::NodePointer> for $name {fn as_ref(&self) -> &$crate::NodePointer{&self.$field
}})*};}/// Implement `Deref<Target=Object>` and `DerefMut<Object>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements Deref<Target=Object> and DerefMut<Object> for MyStruct using field/// // `internal_three_type`/// three_deref_object!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `object`/// three_deref_object!(MyStruct); // equivalent to three_deref_object!(MyStruct#object);/// ```#[macro_export]macro_rules! three_deref_object {($($name:ident),*) => {
three_deref_object!($($name#object),*);};($($name:ident # $field:ident),*) => {
$(impl::std::ops::Dereffor $name {typeTarget = Object;fn deref(&self) -> &Object{&self.$field
}}impl::std::ops::DerefMutfor $name {fn deref_mut(&mutself) -> &mutObject{&mutself.$field
}})*};}

@sunjay

Copy link
Copy Markdown
ContributorAuthor

Looks great in the docs!

screenshot from 2017-06-24 00-59-27

Detailed docs:

screenshot from 2017-06-24 00-59-15

Comment threadsrc/macros.rs Outdated
/// * `Deref<Target=Object>`
/// * `DerefMut<Object>`
///
/// ```

@vitvakatuvitvakatuJun 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Replace this line with

```rust,no_run

to prevent running this as doc test (causes error during Travis CI)

@kvarkkvark left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a very well made PR, thank you!
You included extensive info on the reasoning, great comments, structure, and even provided the doc screenshots, very nice!

Comment threadsrc/macros.rs
/// three_object_wrapper!(MyStruct); // equivalent to three_object_wrapper!(MyStruct#object);
/// ```
#[macro_export]
macro_rules! three_object_wrapper {

@kvarkkvarkJun 24, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm unsure about including "three_" as a macro prefix. Do you see it as an established convention anywhere?

Edit: nevermind, it is the convention, all good!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I don't think it is. The reason I added it is because it's nice to know where macros come from. If you look at the code where I used this (linked in the PR), it's very clear that the macro being used is for this library and its types.

I'm open to another name too if you have something in mind. :)

@kvark

Copy link
Copy Markdown
Collaborator

Another thing that bothers me a bit is the use of #: while it looks ok in the code, it's not near any existing convention (as you mentioned, . or ::). I think ::, while used for methods, is really straightforward here, given that C++ has it for fields as well.

@sunjay

Copy link
Copy Markdown
ContributorAuthor

@vitvakatu@kvark I changed # to :: and added ignore to the doctest since getting it to compile with no_run was quite a bit of extra work. Adding a doctest for this would be great future work!

Let me know if you have any comments. If not, this should be good to go!

@kvark

kvark commented Jun 24, 2017 via email

Copy link
Copy Markdown
Collaborator

borsBot added a commit that referenced this pull request Jun 24, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
@bors

borsBot commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

Build succeeded

@bors
borsBot merged commit c94a970 into three-rs:masterJun 24, 2017
kvark pushed a commit that referenced this pull request Nov 10, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
Sign up for freeto 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.

3 participants

@sunjay@kvark@vitvakatu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Added macro for implementing conversion traits on structs that wrap three-rs types. - #40

Merged
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master
Jun 24, 2017
Merged

Added macro for implementing conversion traits on structs that wrap three-rs types.#40
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master

Conversation

@sunjay

@sunjaysunjay commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.

The macro is nice and flexible and the field name is optional. I chose the syntax MyStruct#field because it seemed more natural to me than MyStruct.field or MyStruct::field (since x::y is usually for modules or methods, not fields).

See the effect of this change here

Really wanted to replace macros in src/scene.rs with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the node field, structs external to the crate do not. This causes problems in as_ref and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.

…hree-rs types.
Really wanted to replace macros in `src/scene.rs` with some macros that
could be used outside of the crate, but unfortunately that didn't workout
because while internal three objects have access to the `node` field,
structs external to the crate do not. This causes problems in `as_ref` and
if that one was going to deviate, we might as well make a single macro for
the special purpose of classes that wrap three-rs objects.
@sunjay

Copy link
Copy Markdown
ContributorAuthor

In case you're curious, my original attempt which would have been able to replace the macros in src/scene.rs is below.

The problem is in AsRef. Just &self.$field causes problems outside of this crate. Specifically, for my project, I get the error:

error[E0308]: mismatched types
--> src/player.rs:11:1
|
11 | three_as_ref_node_pointer!(Player#group);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `froggy::Pointer`, found struct `three::Group`
|
= note: expected type `&froggy::Pointer<three::Node>`
found type `&three::Group`
= note: this error originates in a macro outside of the current crate

In the macro I implemented for this PR, I had to use &self.$field.as_ref() in order for it to work outside of this crate.

/// Implement `AsRef<NodePointer>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements AsRef<NodePointer> for MyStruct using field `internal_three_type`/// three_as_ref_node_pointer!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `node`/// three_as_ref_node_pointer!(MyStruct); // equivalent to three_as_ref_node_pointer!(MyStruct#node);/// ```#[macro_export]macro_rules! three_as_ref_node_pointer {($($name:ident),*) => {
three_as_ref_node_pointer!($($name#node),*);};($($name:ident # $field:ident),*) => {
$(implAsRef<$crate::NodePointer> for $name {fn as_ref(&self) -> &$crate::NodePointer{&self.$field
}})*};}/// Implement `Deref<Target=Object>` and `DerefMut<Object>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements Deref<Target=Object> and DerefMut<Object> for MyStruct using field/// // `internal_three_type`/// three_deref_object!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `object`/// three_deref_object!(MyStruct); // equivalent to three_deref_object!(MyStruct#object);/// ```#[macro_export]macro_rules! three_deref_object {($($name:ident),*) => {
three_deref_object!($($name#object),*);};($($name:ident # $field:ident),*) => {
$(impl::std::ops::Dereffor $name {typeTarget = Object;fn deref(&self) -> &Object{&self.$field
}}impl::std::ops::DerefMutfor $name {fn deref_mut(&mutself) -> &mutObject{&mutself.$field
}})*};}

@sunjay

Copy link
Copy Markdown
ContributorAuthor

Looks great in the docs!

screenshot from 2017-06-24 00-59-27

Detailed docs:

screenshot from 2017-06-24 00-59-15

Comment threadsrc/macros.rs Outdated
/// * `Deref<Target=Object>`
/// * `DerefMut<Object>`
///
/// ```

@vitvakatuvitvakatuJun 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Replace this line with

```rust,no_run

to prevent running this as doc test (causes error during Travis CI)

@kvarkkvark left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a very well made PR, thank you!
You included extensive info on the reasoning, great comments, structure, and even provided the doc screenshots, very nice!

Comment threadsrc/macros.rs
/// three_object_wrapper!(MyStruct); // equivalent to three_object_wrapper!(MyStruct#object);
/// ```
#[macro_export]
macro_rules! three_object_wrapper {

@kvarkkvarkJun 24, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm unsure about including "three_" as a macro prefix. Do you see it as an established convention anywhere?

Edit: nevermind, it is the convention, all good!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I don't think it is. The reason I added it is because it's nice to know where macros come from. If you look at the code where I used this (linked in the PR), it's very clear that the macro being used is for this library and its types.

I'm open to another name too if you have something in mind. :)

@kvark

Copy link
Copy Markdown
Collaborator

Another thing that bothers me a bit is the use of #: while it looks ok in the code, it's not near any existing convention (as you mentioned, . or ::). I think ::, while used for methods, is really straightforward here, given that C++ has it for fields as well.

@sunjay

Copy link
Copy Markdown
ContributorAuthor

@vitvakatu@kvark I changed # to :: and added ignore to the doctest since getting it to compile with no_run was quite a bit of extra work. Adding a doctest for this would be great future work!

Let me know if you have any comments. If not, this should be good to go!

@kvark

kvark commented Jun 24, 2017 via email

Copy link
Copy Markdown
Collaborator

borsBot added a commit that referenced this pull request Jun 24, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
@bors

borsBot commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

Build succeeded

@bors
borsBot merged commit c94a970 into three-rs:masterJun 24, 2017
kvark pushed a commit that referenced this pull request Nov 10, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
Sign up for freeto 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.

3 participants

@sunjay@kvark@vitvakatu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Added macro for implementing conversion traits on structs that wrap three-rs types. - #40

Merged
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master
Jun 24, 2017
Merged

Added macro for implementing conversion traits on structs that wrap three-rs types.#40
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master

Conversation

@sunjay

@sunjaysunjay commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.

The macro is nice and flexible and the field name is optional. I chose the syntax MyStruct#field because it seemed more natural to me than MyStruct.field or MyStruct::field (since x::y is usually for modules or methods, not fields).

See the effect of this change here

Really wanted to replace macros in src/scene.rs with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the node field, structs external to the crate do not. This causes problems in as_ref and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.

…hree-rs types.
Really wanted to replace macros in `src/scene.rs` with some macros that
could be used outside of the crate, but unfortunately that didn't workout
because while internal three objects have access to the `node` field,
structs external to the crate do not. This causes problems in `as_ref` and
if that one was going to deviate, we might as well make a single macro for
the special purpose of classes that wrap three-rs objects.
@sunjay

Copy link
Copy Markdown
ContributorAuthor

In case you're curious, my original attempt which would have been able to replace the macros in src/scene.rs is below.

The problem is in AsRef. Just &self.$field causes problems outside of this crate. Specifically, for my project, I get the error:

error[E0308]: mismatched types
--> src/player.rs:11:1
|
11 | three_as_ref_node_pointer!(Player#group);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `froggy::Pointer`, found struct `three::Group`
|
= note: expected type `&froggy::Pointer<three::Node>`
found type `&three::Group`
= note: this error originates in a macro outside of the current crate

In the macro I implemented for this PR, I had to use &self.$field.as_ref() in order for it to work outside of this crate.

/// Implement `AsRef<NodePointer>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements AsRef<NodePointer> for MyStruct using field `internal_three_type`/// three_as_ref_node_pointer!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `node`/// three_as_ref_node_pointer!(MyStruct); // equivalent to three_as_ref_node_pointer!(MyStruct#node);/// ```#[macro_export]macro_rules! three_as_ref_node_pointer {($($name:ident),*) => {
three_as_ref_node_pointer!($($name#node),*);};($($name:ident # $field:ident),*) => {
$(implAsRef<$crate::NodePointer> for $name {fn as_ref(&self) -> &$crate::NodePointer{&self.$field
}})*};}/// Implement `Deref<Target=Object>` and `DerefMut<Object>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements Deref<Target=Object> and DerefMut<Object> for MyStruct using field/// // `internal_three_type`/// three_deref_object!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `object`/// three_deref_object!(MyStruct); // equivalent to three_deref_object!(MyStruct#object);/// ```#[macro_export]macro_rules! three_deref_object {($($name:ident),*) => {
three_deref_object!($($name#object),*);};($($name:ident # $field:ident),*) => {
$(impl::std::ops::Dereffor $name {typeTarget = Object;fn deref(&self) -> &Object{&self.$field
}}impl::std::ops::DerefMutfor $name {fn deref_mut(&mutself) -> &mutObject{&mutself.$field
}})*};}

@sunjay

Copy link
Copy Markdown
ContributorAuthor

Looks great in the docs!

screenshot from 2017-06-24 00-59-27

Detailed docs:

screenshot from 2017-06-24 00-59-15

Comment threadsrc/macros.rs Outdated
/// * `Deref<Target=Object>`
/// * `DerefMut<Object>`
///
/// ```

@vitvakatuvitvakatuJun 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Replace this line with

```rust,no_run

to prevent running this as doc test (causes error during Travis CI)

@kvarkkvark left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a very well made PR, thank you!
You included extensive info on the reasoning, great comments, structure, and even provided the doc screenshots, very nice!

Comment threadsrc/macros.rs
/// three_object_wrapper!(MyStruct); // equivalent to three_object_wrapper!(MyStruct#object);
/// ```
#[macro_export]
macro_rules! three_object_wrapper {

@kvarkkvarkJun 24, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm unsure about including "three_" as a macro prefix. Do you see it as an established convention anywhere?

Edit: nevermind, it is the convention, all good!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I don't think it is. The reason I added it is because it's nice to know where macros come from. If you look at the code where I used this (linked in the PR), it's very clear that the macro being used is for this library and its types.

I'm open to another name too if you have something in mind. :)

@kvark

Copy link
Copy Markdown
Collaborator

Another thing that bothers me a bit is the use of #: while it looks ok in the code, it's not near any existing convention (as you mentioned, . or ::). I think ::, while used for methods, is really straightforward here, given that C++ has it for fields as well.

@sunjay

Copy link
Copy Markdown
ContributorAuthor

@vitvakatu@kvark I changed # to :: and added ignore to the doctest since getting it to compile with no_run was quite a bit of extra work. Adding a doctest for this would be great future work!

Let me know if you have any comments. If not, this should be good to go!

@kvark

kvark commented Jun 24, 2017 via email

Copy link
Copy Markdown
Collaborator

borsBot added a commit that referenced this pull request Jun 24, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
@bors

borsBot commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

Build succeeded

@bors
borsBot merged commit c94a970 into three-rs:masterJun 24, 2017
kvark pushed a commit that referenced this pull request Nov 10, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
Sign up for freeto 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.

3 participants

@sunjay@kvark@vitvakatu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Added macro for implementing conversion traits on structs that wrap three-rs types. - #40

Merged
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master
Jun 24, 2017
Merged

Added macro for implementing conversion traits on structs that wrap three-rs types.#40
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master

Conversation

@sunjay

@sunjaysunjay commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.

The macro is nice and flexible and the field name is optional. I chose the syntax MyStruct#field because it seemed more natural to me than MyStruct.field or MyStruct::field (since x::y is usually for modules or methods, not fields).

See the effect of this change here

Really wanted to replace macros in src/scene.rs with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the node field, structs external to the crate do not. This causes problems in as_ref and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.

…hree-rs types.
Really wanted to replace macros in `src/scene.rs` with some macros that
could be used outside of the crate, but unfortunately that didn't workout
because while internal three objects have access to the `node` field,
structs external to the crate do not. This causes problems in `as_ref` and
if that one was going to deviate, we might as well make a single macro for
the special purpose of classes that wrap three-rs objects.
@sunjay

Copy link
Copy Markdown
ContributorAuthor

In case you're curious, my original attempt which would have been able to replace the macros in src/scene.rs is below.

The problem is in AsRef. Just &self.$field causes problems outside of this crate. Specifically, for my project, I get the error:

error[E0308]: mismatched types
--> src/player.rs:11:1
|
11 | three_as_ref_node_pointer!(Player#group);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `froggy::Pointer`, found struct `three::Group`
|
= note: expected type `&froggy::Pointer<three::Node>`
found type `&three::Group`
= note: this error originates in a macro outside of the current crate

In the macro I implemented for this PR, I had to use &self.$field.as_ref() in order for it to work outside of this crate.

/// Implement `AsRef<NodePointer>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements AsRef<NodePointer> for MyStruct using field `internal_three_type`/// three_as_ref_node_pointer!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `node`/// three_as_ref_node_pointer!(MyStruct); // equivalent to three_as_ref_node_pointer!(MyStruct#node);/// ```#[macro_export]macro_rules! three_as_ref_node_pointer {($($name:ident),*) => {
three_as_ref_node_pointer!($($name#node),*);};($($name:ident # $field:ident),*) => {
$(implAsRef<$crate::NodePointer> for $name {fn as_ref(&self) -> &$crate::NodePointer{&self.$field
}})*};}/// Implement `Deref<Target=Object>` and `DerefMut<Object>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements Deref<Target=Object> and DerefMut<Object> for MyStruct using field/// // `internal_three_type`/// three_deref_object!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `object`/// three_deref_object!(MyStruct); // equivalent to three_deref_object!(MyStruct#object);/// ```#[macro_export]macro_rules! three_deref_object {($($name:ident),*) => {
three_deref_object!($($name#object),*);};($($name:ident # $field:ident),*) => {
$(impl::std::ops::Dereffor $name {typeTarget = Object;fn deref(&self) -> &Object{&self.$field
}}impl::std::ops::DerefMutfor $name {fn deref_mut(&mutself) -> &mutObject{&mutself.$field
}})*};}

@sunjay

Copy link
Copy Markdown
ContributorAuthor

Looks great in the docs!

screenshot from 2017-06-24 00-59-27

Detailed docs:

screenshot from 2017-06-24 00-59-15

Comment threadsrc/macros.rs Outdated
/// * `Deref<Target=Object>`
/// * `DerefMut<Object>`
///
/// ```

@vitvakatuvitvakatuJun 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Replace this line with

```rust,no_run

to prevent running this as doc test (causes error during Travis CI)

@kvarkkvark left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a very well made PR, thank you!
You included extensive info on the reasoning, great comments, structure, and even provided the doc screenshots, very nice!

Comment threadsrc/macros.rs
/// three_object_wrapper!(MyStruct); // equivalent to three_object_wrapper!(MyStruct#object);
/// ```
#[macro_export]
macro_rules! three_object_wrapper {

@kvarkkvarkJun 24, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm unsure about including "three_" as a macro prefix. Do you see it as an established convention anywhere?

Edit: nevermind, it is the convention, all good!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I don't think it is. The reason I added it is because it's nice to know where macros come from. If you look at the code where I used this (linked in the PR), it's very clear that the macro being used is for this library and its types.

I'm open to another name too if you have something in mind. :)

@kvark

Copy link
Copy Markdown
Collaborator

Another thing that bothers me a bit is the use of #: while it looks ok in the code, it's not near any existing convention (as you mentioned, . or ::). I think ::, while used for methods, is really straightforward here, given that C++ has it for fields as well.

@sunjay

Copy link
Copy Markdown
ContributorAuthor

@vitvakatu@kvark I changed # to :: and added ignore to the doctest since getting it to compile with no_run was quite a bit of extra work. Adding a doctest for this would be great future work!

Let me know if you have any comments. If not, this should be good to go!

@kvark

kvark commented Jun 24, 2017 via email

Copy link
Copy Markdown
Collaborator

borsBot added a commit that referenced this pull request Jun 24, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
@bors

borsBot commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

Build succeeded

@bors
borsBot merged commit c94a970 into three-rs:masterJun 24, 2017
kvark pushed a commit that referenced this pull request Nov 10, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
Sign up for freeto 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.

3 participants

@sunjay@kvark@vitvakatu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Added macro for implementing conversion traits on structs that wrap three-rs types. - #40

Merged
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master
Jun 24, 2017
Merged

Added macro for implementing conversion traits on structs that wrap three-rs types.#40
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master

Conversation

@sunjay

@sunjaysunjay commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.

The macro is nice and flexible and the field name is optional. I chose the syntax MyStruct#field because it seemed more natural to me than MyStruct.field or MyStruct::field (since x::y is usually for modules or methods, not fields).

See the effect of this change here

Really wanted to replace macros in src/scene.rs with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the node field, structs external to the crate do not. This causes problems in as_ref and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.

…hree-rs types.
Really wanted to replace macros in `src/scene.rs` with some macros that
could be used outside of the crate, but unfortunately that didn't workout
because while internal three objects have access to the `node` field,
structs external to the crate do not. This causes problems in `as_ref` and
if that one was going to deviate, we might as well make a single macro for
the special purpose of classes that wrap three-rs objects.
@sunjay

Copy link
Copy Markdown
ContributorAuthor

In case you're curious, my original attempt which would have been able to replace the macros in src/scene.rs is below.

The problem is in AsRef. Just &self.$field causes problems outside of this crate. Specifically, for my project, I get the error:

error[E0308]: mismatched types
--> src/player.rs:11:1
|
11 | three_as_ref_node_pointer!(Player#group);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `froggy::Pointer`, found struct `three::Group`
|
= note: expected type `&froggy::Pointer<three::Node>`
found type `&three::Group`
= note: this error originates in a macro outside of the current crate

In the macro I implemented for this PR, I had to use &self.$field.as_ref() in order for it to work outside of this crate.

/// Implement `AsRef<NodePointer>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements AsRef<NodePointer> for MyStruct using field `internal_three_type`/// three_as_ref_node_pointer!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `node`/// three_as_ref_node_pointer!(MyStruct); // equivalent to three_as_ref_node_pointer!(MyStruct#node);/// ```#[macro_export]macro_rules! three_as_ref_node_pointer {($($name:ident),*) => {
three_as_ref_node_pointer!($($name#node),*);};($($name:ident # $field:ident),*) => {
$(implAsRef<$crate::NodePointer> for $name {fn as_ref(&self) -> &$crate::NodePointer{&self.$field
}})*};}/// Implement `Deref<Target=Object>` and `DerefMut<Object>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements Deref<Target=Object> and DerefMut<Object> for MyStruct using field/// // `internal_three_type`/// three_deref_object!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `object`/// three_deref_object!(MyStruct); // equivalent to three_deref_object!(MyStruct#object);/// ```#[macro_export]macro_rules! three_deref_object {($($name:ident),*) => {
three_deref_object!($($name#object),*);};($($name:ident # $field:ident),*) => {
$(impl::std::ops::Dereffor $name {typeTarget = Object;fn deref(&self) -> &Object{&self.$field
}}impl::std::ops::DerefMutfor $name {fn deref_mut(&mutself) -> &mutObject{&mutself.$field
}})*};}

@sunjay

Copy link
Copy Markdown
ContributorAuthor

Looks great in the docs!

screenshot from 2017-06-24 00-59-27

Detailed docs:

screenshot from 2017-06-24 00-59-15

Comment threadsrc/macros.rs Outdated
/// * `Deref<Target=Object>`
/// * `DerefMut<Object>`
///
/// ```

@vitvakatuvitvakatuJun 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Replace this line with

```rust,no_run

to prevent running this as doc test (causes error during Travis CI)

@kvarkkvark left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a very well made PR, thank you!
You included extensive info on the reasoning, great comments, structure, and even provided the doc screenshots, very nice!

Comment threadsrc/macros.rs
/// three_object_wrapper!(MyStruct); // equivalent to three_object_wrapper!(MyStruct#object);
/// ```
#[macro_export]
macro_rules! three_object_wrapper {

@kvarkkvarkJun 24, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm unsure about including "three_" as a macro prefix. Do you see it as an established convention anywhere?

Edit: nevermind, it is the convention, all good!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I don't think it is. The reason I added it is because it's nice to know where macros come from. If you look at the code where I used this (linked in the PR), it's very clear that the macro being used is for this library and its types.

I'm open to another name too if you have something in mind. :)

@kvark

Copy link
Copy Markdown
Collaborator

Another thing that bothers me a bit is the use of #: while it looks ok in the code, it's not near any existing convention (as you mentioned, . or ::). I think ::, while used for methods, is really straightforward here, given that C++ has it for fields as well.

@sunjay

Copy link
Copy Markdown
ContributorAuthor

@vitvakatu@kvark I changed # to :: and added ignore to the doctest since getting it to compile with no_run was quite a bit of extra work. Adding a doctest for this would be great future work!

Let me know if you have any comments. If not, this should be good to go!

@kvark

kvark commented Jun 24, 2017 via email

Copy link
Copy Markdown
Collaborator

borsBot added a commit that referenced this pull request Jun 24, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
@bors

borsBot commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

Build succeeded

@bors
borsBot merged commit c94a970 into three-rs:masterJun 24, 2017
kvark pushed a commit that referenced this pull request Nov 10, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
Sign up for freeto 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.

3 participants

@sunjay@kvark@vitvakatu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Added macro for implementing conversion traits on structs that wrap three-rs types. - #40

Merged
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master
Jun 24, 2017
Merged

Added macro for implementing conversion traits on structs that wrap three-rs types.#40
bors[bot] merged 2 commits into
three-rs:masterfrom
sunjay:master

Conversation

@sunjay

@sunjaysunjay commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.

The macro is nice and flexible and the field name is optional. I chose the syntax MyStruct#field because it seemed more natural to me than MyStruct.field or MyStruct::field (since x::y is usually for modules or methods, not fields).

See the effect of this change here

Really wanted to replace macros in src/scene.rs with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the node field, structs external to the crate do not. This causes problems in as_ref and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.

…hree-rs types.
Really wanted to replace macros in `src/scene.rs` with some macros that
could be used outside of the crate, but unfortunately that didn't workout
because while internal three objects have access to the `node` field,
structs external to the crate do not. This causes problems in `as_ref` and
if that one was going to deviate, we might as well make a single macro for
the special purpose of classes that wrap three-rs objects.
@sunjay

Copy link
Copy Markdown
ContributorAuthor

In case you're curious, my original attempt which would have been able to replace the macros in src/scene.rs is below.

The problem is in AsRef. Just &self.$field causes problems outside of this crate. Specifically, for my project, I get the error:

error[E0308]: mismatched types
--> src/player.rs:11:1
|
11 | three_as_ref_node_pointer!(Player#group);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `froggy::Pointer`, found struct `three::Group`
|
= note: expected type `&froggy::Pointer<three::Node>`
found type `&three::Group`
= note: this error originates in a macro outside of the current crate

In the macro I implemented for this PR, I had to use &self.$field.as_ref() in order for it to work outside of this crate.

/// Implement `AsRef<NodePointer>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements AsRef<NodePointer> for MyStruct using field `internal_three_type`/// three_as_ref_node_pointer!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `node`/// three_as_ref_node_pointer!(MyStruct); // equivalent to three_as_ref_node_pointer!(MyStruct#node);/// ```#[macro_export]macro_rules! three_as_ref_node_pointer {($($name:ident),*) => {
three_as_ref_node_pointer!($($name#node),*);};($($name:ident # $field:ident),*) => {
$(implAsRef<$crate::NodePointer> for $name {fn as_ref(&self) -> &$crate::NodePointer{&self.$field
}})*};}/// Implement `Deref<Target=Object>` and `DerefMut<Object>` for a given type./// Useful for when you wrap a three-rs type with your own struct. Allows you to/// use that struct in place of any three-rs object.////// ```/// // Implements Deref<Target=Object> and DerefMut<Object> for MyStruct using field/// // `internal_three_type`/// three_deref_object!(MyStruct#internal_three_type);/// // If field is omitted, it defaults to `object`/// three_deref_object!(MyStruct); // equivalent to three_deref_object!(MyStruct#object);/// ```#[macro_export]macro_rules! three_deref_object {($($name:ident),*) => {
three_deref_object!($($name#object),*);};($($name:ident # $field:ident),*) => {
$(impl::std::ops::Dereffor $name {typeTarget = Object;fn deref(&self) -> &Object{&self.$field
}}impl::std::ops::DerefMutfor $name {fn deref_mut(&mutself) -> &mutObject{&mutself.$field
}})*};}

@sunjay

Copy link
Copy Markdown
ContributorAuthor

Looks great in the docs!

screenshot from 2017-06-24 00-59-27

Detailed docs:

screenshot from 2017-06-24 00-59-15

Comment threadsrc/macros.rs Outdated
/// * `Deref<Target=Object>`
/// * `DerefMut<Object>`
///
/// ```

@vitvakatuvitvakatuJun 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Replace this line with

```rust,no_run

to prevent running this as doc test (causes error during Travis CI)

@kvarkkvark left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a very well made PR, thank you!
You included extensive info on the reasoning, great comments, structure, and even provided the doc screenshots, very nice!

Comment threadsrc/macros.rs
/// three_object_wrapper!(MyStruct); // equivalent to three_object_wrapper!(MyStruct#object);
/// ```
#[macro_export]
macro_rules! three_object_wrapper {

@kvarkkvarkJun 24, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm unsure about including "three_" as a macro prefix. Do you see it as an established convention anywhere?

Edit: nevermind, it is the convention, all good!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I don't think it is. The reason I added it is because it's nice to know where macros come from. If you look at the code where I used this (linked in the PR), it's very clear that the macro being used is for this library and its types.

I'm open to another name too if you have something in mind. :)

@kvark

Copy link
Copy Markdown
Collaborator

Another thing that bothers me a bit is the use of #: while it looks ok in the code, it's not near any existing convention (as you mentioned, . or ::). I think ::, while used for methods, is really straightforward here, given that C++ has it for fields as well.

@sunjay

Copy link
Copy Markdown
ContributorAuthor

@vitvakatu@kvark I changed # to :: and added ignore to the doctest since getting it to compile with no_run was quite a bit of extra work. Adding a doctest for this would be great future work!

Let me know if you have any comments. If not, this should be good to go!

@kvark

kvark commented Jun 24, 2017 via email

Copy link
Copy Markdown
Collaborator

borsBot added a commit that referenced this pull request Jun 24, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
@bors

borsBot commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

Build succeeded

@bors
borsBot merged commit c94a970 into three-rs:masterJun 24, 2017
kvark pushed a commit that referenced this pull request Nov 10, 2017
40: Added macro for implementing conversion traits on structs that wrap three-rs types. r=kvark
This macro makes it really easy for structs that simply wrap a three-rs type to implement the right traits so that those custom types can interop with three-rs methods really easily.
The macro is nice and flexible and the field name is optional. I chose the syntax `MyStruct#field` because it seemed more natural to me than `MyStruct.field` or `MyStruct::field` (since `x::y` is usually for modules or methods, not fields).
[See the effect of this change here](https://github.com/sunjay/panda/commit/d508806d9f5f47fe4267b98f5823142108decaac)
Really wanted to replace macros in `src/scene.rs` with some macros that could be used outside of the crate, but unfortunately that didn't workout because while internal three objects have access to the `node` field, structs external to the crate do not. This causes problems in `as_ref` and if that one was going to deviate, we might as well make a single macro for the special purpose of classes that wrap three-rs objects.
Sign up for freeto 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.

3 participants

@sunjay@kvark@vitvakatu