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

Don't check for Tool getExtents if tool only draws 2D #118

Open
Eneroth3 opened this issue May 7, 2019 · 1 comment
Open

Don't check for Tool getExtents if tool only draws 2D #118

Eneroth3 opened this issue May 7, 2019 · 1 comment

Comments

@Eneroth3
Copy link
Contributor

Eneroth3 commented May 7, 2019

Currently the SketchupSuggestions/ToolDrawingBounds cop catches tools that has a draw method but no getExtents method. However, tools that only draw in 2D have no use of a setting any 3D extents to draw to.

Perhaps the cop could also check what View draw methods are called inside the draw method? If only draw2d was called, there is no need for a getExtents.

This may not be reliable if the view draw calls are wrapped in other methods that in turn are called from the Tool interface draw method.

@thomthom
Copy link
Member

thomthom commented May 7, 2019

The cops in SketchupSuggestion category is likely to be more noisy. Some of them are there mostly to be part of the learning process and developers can simply mute it once they don't feel they don't need it.

What we see in reviews is that a lot of the times people don't use getExtents at all. We could check for draw2d directly in the draw callback - but it'll add complexity that only cover a small set of scenarios. It won't catch drawing helper methods.

We can expand the documentation to be a bit more explicit about the cases of drawing only 2d - like suggesting the cop be disabled completely for the project, per file, or even just for that given tool class.

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

No branches or pull requests

2 participants