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

Document that Sketchup::PickHelper instances are reused #198

Open
thomthom opened this Issue Feb 11, 2019 · 7 comments

Comments

Projects
None yet
3 participants
@thomthom
Copy link
Member

thomthom commented Feb 11, 2019

Reference:

thomthom/true-bend#24
https://forums.sketchup.com/t/truebend-re-defined-method-for-pick-segment-points/87777/13

Sketchup::PickHelper objects are reused per view. view.pick_helper does not return a fresh PickHelper.
This should be clarified in the documentation along with a note that instances of this class should not be extended or otherwise modified.

@Eneroth3

This comment has been minimized.

Copy link

Eneroth3 commented Feb 11, 2019

I'm thinking that rather making a note in the documentation of this method, there could maybe be a more comprehensive document of what objects are safe to extend and which ones are not. This would include all Entity objects (I think) put not points, vectors and transformations returned by the API.

@thomthom

This comment has been minimized.

Copy link
Member Author

thomthom commented Feb 11, 2019

Yea, Sketchup::Entity is reused via the object manager we have. All others should be fine - except it turned out that Sketchup::PickHelper had its own management.

@DanRathbun

This comment has been minimized.

Copy link

DanRathbun commented Feb 11, 2019

Isn’t there a way to mark Singleton classes as singleton in YARD ?

View and Selection are also singleton.

@thomthom

This comment has been minimized.

Copy link
Member Author

thomthom commented Feb 12, 2019

Not nativity. But it's possible to add custom tags, and then add custom tag handlers. So it's possible to introduce a new tag that the template understands and then add some formatting/text.

@DanRathbun

This comment has been minimized.

Copy link

DanRathbun commented Feb 13, 2019

Not nativity.

I might have been thinking of the @abstract tag.

The @!parse directive looks interesting. This would likely cause YARD to add in methods that the SketchUp API does not ...

class Sketchup::PickHelper
  # @!parse include Singleton
end

The @!parse tag can also parse C snippet comments. See docs.

But it's possible to add custom tags, and then add custom tag handlers. So it's possible to introduce a new tag that the template understands and then add some formatting/text.

Probably the best path in the long run. An @note might suffice in the short term.

@thomthom

This comment has been minimized.

Copy link
Member Author

thomthom commented Feb 13, 2019

Saying that Entity or View is a Singleton is misleading - because that mix-in is a pattern where you obtain the singleton instance via a .instance method on the class. That's not the case for the SU API "singletons". Their constructor is part of another class. So we need to find another way to document this, like with custom tags.

@DanRathbun

This comment has been minimized.

Copy link

DanRathbun commented Feb 16, 2019

I implied this above ...

This would likely cause YARD to add in methods that the SketchUp API does not ...

The #instance method was the main one I meant.

Their constructor is part of another class. So we need to find another way to document this, like with custom tags.

And I agreed above custom tags are likely the best idea.

HOWEVER, they are not just objects generated via factory methods that hide a constructor, they are actual singleton objects exposed via a getter method. (Ie, the result of the getter is never a new factory generated instance, it is always the same object per model.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment