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

Move Bidi static methods to instance methods on TextDirection #334

Closed
DartBot opened this issue Jun 5, 2015 · 4 comments
Closed

Move Bidi static methods to instance methods on TextDirection #334

DartBot opened this issue Jun 5, 2015 · 4 comments
Labels
closed-not-planned Closed as we don't intend to take action on the reported issue type-enhancement A request for a change that isn't a bug

Comments

@DartBot
Copy link

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/444270?v=3" align="left" width="96" height="96"hspace="10"> Issue by seaneagan
Originally opened as dart-lang/sdk#11752


In package:intl, Bidi acts as a namespace for static methods, which is generally an anti-pattern in dart, since dart libraries can act as namespaces for static methods, so a separate "bidi.dart" library maybe. However, most of these methods are duplicated for both TextDirections (RTL and LTR), so it seems like making them polymorphic to TextDirection would make sense. For a basic idea of what this would look like, see:

https://gist.github.com/seaneagan/5957736

Of course they wouldn't make sense for TextDirection.UNKNOWN, but I don't think TextDirection.UNKNOWN is necessary. I think it would make more sense just to reuse null for that concept.

Another option would be to keep them as static/top-level methods which receive a TextDirection, but that doesn't feel quite as nice.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/5449880?v=3" align="left" width="48" height="48"hspace="10"> Comment by iposva-google


Added Area-Pkg, Triaged labels.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/5479?v=3" align="left" width="48" height="48"hspace="10"> Comment by sethladd


cc @alan-knight.
Removed Type-Defect label.
Added Type-Enhancement, Pkg-Intl labels.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/5479?v=3" align="left" width="48" height="48"hspace="10"> Comment by sethladd


Do we intend to do this? If not, can we close?

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/3476088?v=3" align="left" width="48" height="48"hspace="10"> Comment by alan-knight


I'd say close. It's probably a good idea, but I don't think people tend to use those methods explicitly, so it doesn't seem worth the work to re-arrange. We have a lot bigger things that need work.


Added WontFix label.

@DartBot DartBot added type-enhancement A request for a change that isn't a bug closed-not-planned Closed as we don't intend to take action on the reported issue labels Jun 5, 2015
@mosuem mosuem transferred this issue from dart-archive/intl Apr 18, 2023
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-not-planned Closed as we don't intend to take action on the reported issue type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

1 participant