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

Default generate_display_trait #668

Merged
merged 1 commit into from Nov 19, 2018

Conversation

Projects
None yet
3 participants
@EPashkin
Copy link
Member

EPashkin commented Nov 18, 2018

Fix problem in #643 (review)

cc @GuillaumeGomez , @sdroege

@EPashkin

This comment has been minimized.

Copy link
Member Author

EPashkin commented Nov 18, 2018

There also possible to separate defaults for enum and objects, but IMHO it unnecessary,
and will need more complex change

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Nov 18, 2018

You added a global option for it? Sounds actually like a good idea. :) But then, could you precise in the example that if it's not set on an object, then it's set for all objects please?

@EPashkin EPashkin force-pushed the EPashkin:default_generate_display_trait branch from 429550d to 00d10b1 Nov 19, 2018

@EPashkin

This comment has been minimized.

Copy link
Member Author

EPashkin commented Nov 19, 2018

Updated readme.txt

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Nov 19, 2018

Seems fine to me but I'd still prefer it to be disabled by default :) These Display impls we generate are generally not what Display is meant for apart from very few cases

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Nov 19, 2018

I'd prefer to keep it on by default. However, let's vote.

@GuillaumeGomez votes for it.
@sdroege votes against it.

What about you @EPashkin?

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Nov 19, 2018

I should probably add my rationale also :) Display is meant for things to show to the user, but these impls are not providing anything that is useful for users. They are useful for programmers, and that's what Debug is for

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Nov 19, 2018

Maybe but it also allows to use the ToString trait, which is very convenient whereas Debug doesn't. :)

And both Displayt and Debug are for programmers in GUI applications development. In here, they just don't have the same purpose.

@EPashkin

This comment has been minimized.

Copy link
Member Author

EPashkin commented Nov 19, 2018

I also think that Display don't usable for end users for GUI elements anyway,
maybe with exception for some row-like objects or other rare cases,
so generating Display instead Debug is acceptable in this case.
So I vote for generating by default too.

@EPashkin EPashkin merged commit e578e4f into gtk-rs:master Nov 19, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@EPashkin EPashkin deleted the EPashkin:default_generate_display_trait branch Nov 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.