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.

Sign up for GitHub

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

Jump to bottom

Tk overwrites Base.Text #117

Open
pfitzseb opened this issue Feb 29, 2016 · 4 comments · May be fixed by #138
Open

Tk overwrites Base.Text #117

pfitzseb opened this issue Feb 29, 2016 · 4 comments · May be fixed by #138

Comments

@pfitzseb
Copy link

which causes issues when using Atom.jl when importing Tk.jl in user code.

Ref: JunoLab/atom-julia-client#118

The solutions I see would be to either

  • not export Text and require the user to use Tk.Text or maybe define TkText or
  • make Widget not include AbstractString and instead use TkWidget(x::AbstractString) or something similar.

Edit: And since the second half of this issue doesn't appear for mysterious reasons (actually, most likely a forgotten Ctrl+Enter...):

  • use Text{typeof(x)}(x) everywhere in Atom to use the parametric constructor from Base, but that seems oh so ugly. Maybe there's another way to enforce using a specific constructor?

I'm not sure if there's any kind of guideline for overwriting types from Base, but Tk's behaviour seems kinda strange to me. If there's something we can/should change on our side, I'd be glad to do it -- otherwise I'd be happy to implement some change in Tk as well.
@bjarthur bjarthur mentioned this issue Sep 27, 2016
Merged
@Nectarineimp
Copy link

I vote for TkText definition. Currently including Tk breaks way too many other things by overriding Base.Text.

@tkelman
Copy link
Contributor

tkelman commented Apr 21, 2017

Why is Base even exporting Text ? That seems internal to the Markdown parser and not a type users should need to work with very often.

@pfitzseb pfitzseb mentioned this issue May 2, 2017
Closed
@pfitzseb
Copy link
Author

pfitzseb commented May 2, 2017

Right, so should we rename Tk.Text, unexport it from Tk or unexport Text from Base?

@tkelman
Copy link
Contributor

tkelman commented May 2, 2017

All of the above?

@pfitzseb pfitzseb linked a pull request May 2, 2017 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Rename Text to TkText pfitzseb/Tk.jl
3 participants
@Nectarineimp @tkelman @pfitzseb