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

Rename conversion routines, again. #8473

Closed
Ms2ger opened this issue Nov 11, 2015 · 13 comments
Closed

Rename conversion routines, again. #8473

Ms2ger opened this issue Nov 11, 2015 · 13 comments

Comments

@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Nov 11, 2015

In b290a31, @nox renamed a bunch of conversion routines in a way that is almost diametrically opposed to the standard terminology we use. In particular, an object that implements Reflectable is a "DOM object", and a JSObject for a DOM object is its "reflector". We should correct these names.

@nox
Copy link
Member

@nox nox commented Nov 11, 2015

So why would "reflector" refer to JSObject, when we have an actual Rust structure called Reflector and nothing named Object anywhere? We should probably rename again everything, but we need a new term somewhere.

@nox
Copy link
Member

@nox nox commented Nov 11, 2015

In the documentation you point out, I would rename DOM object to DOM structure, and reflector to object.

@Ms2ger
Copy link
Contributor Author

@Ms2ger Ms2ger commented Nov 12, 2015

@bholley
Copy link
Contributor

@bholley bholley commented Nov 12, 2015

"Object Reflection" is the standard term that I'm aware of for mirroring objects across language runtimes. Of those two words, the "reflect" part is much less ambiguous than the "object" part, which is why "reflector" is a better name than "object".

It is unfortunate that Rust has also recently introduced a trait called "Reflect". However, I still think that in there are, in total, several orders of magnitude more meanings of the term "object" than there are of the term "reflector", and that the former would be extremely confusing, especially in the case where a given Rust DOM object holds onto other JS objects in addition to its reflector.

@jdm
Copy link
Member

@jdm jdm commented Nov 12, 2015

Rust introduced a Reflect trait?

@metajack
Copy link
Contributor

@metajack metajack commented Nov 12, 2015

@bholley
Copy link
Contributor

@bholley bholley commented Nov 12, 2015

FWIW the naming of "Reflectable" rather than "Reflect" was my fault, since I was a rust n00b at the time (several years ago). I sorta figured it would get fixed at some point.

@nox
Copy link
Member

@nox nox commented Dec 2, 2015

What about global_root_from_reflectable, instead of global_root_from_reflector?

@Ms2ger
Copy link
Contributor Author

@Ms2ger Ms2ger commented Dec 2, 2015

I think I'd prefer renaming Reflectable to DOMObject.

@bholley
Copy link
Contributor

@bholley bholley commented Dec 2, 2015

I think I'd prefer renaming Reflectable to DOMObject.

That sounds reasonable to me.

@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Dec 8, 2016

I think I'd prefer renaming Reflectable to DOMObject.

Is this change still desired? If so, I can make the change.

@nox
Copy link
Member

@nox nox commented Dec 8, 2016

Seems ok with me too.

frewsxcv added a commit to frewsxcv/servo that referenced this issue Dec 8, 2016
frewsxcv added a commit to frewsxcv/servo that referenced this issue Dec 8, 2016
@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Dec 8, 2016

PR opened: #14495.

🖌 🚲 🏠

bors-servo added a commit that referenced this issue Dec 8, 2016
Rename `Reflectable` to `DomObject`.

Fixes #8473.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14495)
<!-- Reviewable:end -->
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 4, 2017
…xcv:reflectable-domobject); r=jdm

Fixes servo/servo#8473.

Source-Repo: https://github.com/servo/servo
Source-Revision: b192ae9db7082346a4a6a985c5557d4cea75d50e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 1, 2019
…xcv:reflectable-domobject); r=jdm

Fixes servo/servo#8473.

Source-Repo: https://github.com/servo/servo
Source-Revision: b192ae9db7082346a4a6a985c5557d4cea75d50e

UltraBlame original commit: 81364670e29057d38aeea04189822f8dd45275b6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
…xcv:reflectable-domobject); r=jdm

Fixes servo/servo#8473.

Source-Repo: https://github.com/servo/servo
Source-Revision: b192ae9db7082346a4a6a985c5557d4cea75d50e

UltraBlame original commit: 81364670e29057d38aeea04189822f8dd45275b6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
…xcv:reflectable-domobject); r=jdm

Fixes servo/servo#8473.

Source-Repo: https://github.com/servo/servo
Source-Revision: b192ae9db7082346a4a6a985c5557d4cea75d50e

UltraBlame original commit: 81364670e29057d38aeea04189822f8dd45275b6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.