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

Text.fromString references missing module #805

Closed
jszmajda opened this Issue Jan 13, 2017 · 18 comments

Comments

Projects
None yet
5 participants
@jszmajda

jszmajda commented Jan 13, 2017

https://github.com/elm-lang/core/blob/master/src/Basics.elm#L472

This document references Text.fromString, but the Text module is missing and there doesn't seem to be any implementation of fromString anymore in core.

I'm not sure where to direct readers to find the functionality in fromString, else I'd submit a PR.

@process-bot

This comment has been minimized.

Show comment
Hide comment
@process-bot

process-bot Jan 13, 2017

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

process-bot commented Jan 13, 2017

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

@jszmajda

This comment has been minimized.

Show comment
Hide comment
@jszmajda

jszmajda Jan 16, 2017

Is Text.fromString no longer planned to be supported?

jszmajda commented Jan 16, 2017

Is Text.fromString no longer planned to be supported?

@mgold

This comment has been minimized.

Show comment
Hide comment
@mgold

mgold Jan 16, 2017

Contributor

The Text module and type are long gone.

Contributor

mgold commented Jan 16, 2017

The Text module and type are long gone.

@jszmajda

This comment has been minimized.

Show comment
Hide comment
@jszmajda

jszmajda Jan 16, 2017

I meant the ability to convert a string into an Elm symbol.

It would reduce the boilerplate I need in this routing function example:

locationUpdate : Navigation.Location -> App.Msg
locationUpdate location =
  case (String.dropLeft 2 location.hash) of

    "ViewDocList" -> ViewState ViewDocList

    "ViewDocument" -> ViewState ViewDocument

    _ -> App.NoOp

    -- Would rather code this as:
    -- case Text.fromString msg of
    --   Just value -> ViewState value
    --   Nothing    -> App.NoOp

Sure I could just pass the strings along into the ViewState constructor for the App.Msg but somehow that seems contrary to the point of strongly-typed code.

jszmajda commented Jan 16, 2017

I meant the ability to convert a string into an Elm symbol.

It would reduce the boilerplate I need in this routing function example:

locationUpdate : Navigation.Location -> App.Msg
locationUpdate location =
  case (String.dropLeft 2 location.hash) of

    "ViewDocList" -> ViewState ViewDocList

    "ViewDocument" -> ViewState ViewDocument

    _ -> App.NoOp

    -- Would rather code this as:
    -- case Text.fromString msg of
    --   Just value -> ViewState value
    --   Nothing    -> App.NoOp

Sure I could just pass the strings along into the ViewState constructor for the App.Msg but somehow that seems contrary to the point of strongly-typed code.

@mgold

This comment has been minimized.

Show comment
Hide comment
@mgold

mgold Jan 16, 2017

Contributor

There are ways of doing what you want and you should ask on the mailing list. What you're asking about is not what Text.fromString used to do.

Contributor

mgold commented Jan 16, 2017

There are ways of doing what you want and you should ask on the mailing list. What you're asking about is not what Text.fromString used to do.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Jan 16, 2017

Contributor

The Text module and type are long gone.

Actually, they are moved: http://package.elm-lang.org/packages/evancz/elm-graphics/1.0.1/Text#fromString

Contributor

jvoigtlaender commented Jan 16, 2017

The Text module and type are long gone.

Actually, they are moved: http://package.elm-lang.org/packages/evancz/elm-graphics/1.0.1/Text#fromString

@jszmajda

This comment has been minimized.

Show comment
Hide comment
@jszmajda

jszmajda Jan 16, 2017

Text.fromString in evancz/elm-graphics has different functionality (it converts String -> Text when I'm looking for String -> a)

I'll ask on the mailing list, thanks!

jszmajda commented Jan 16, 2017

Text.fromString in evancz/elm-graphics has different functionality (it converts String -> Text when I'm looking for String -> a)

I'll ask on the mailing list, thanks!

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Jan 16, 2017

Contributor

I know that it has different functionality. I provided the pointer to it so that you can know what functionality it has (in contrast to what you apparently believed its functionality to be - and you saying you don't know where to find it).

In any case, the Text.fromString in evancz/elm-graphics is exactly the Text.fromString that the line https://github.com/elm-lang/core/blob/master/src/Basics.elm#L472 you pointed to is referring to. That line still lives on from when the Text.fromString that is now in evancz/elm-graphics was living inside elm-lang/core. So one actual fix to the issue you have opened here would indeed be to mention in the comment that the Text.fromString referred to can be found in evancz/elm-graphics.

Contributor

jvoigtlaender commented Jan 16, 2017

I know that it has different functionality. I provided the pointer to it so that you can know what functionality it has (in contrast to what you apparently believed its functionality to be - and you saying you don't know where to find it).

In any case, the Text.fromString in evancz/elm-graphics is exactly the Text.fromString that the line https://github.com/elm-lang/core/blob/master/src/Basics.elm#L472 you pointed to is referring to. That line still lives on from when the Text.fromString that is now in evancz/elm-graphics was living inside elm-lang/core. So one actual fix to the issue you have opened here would indeed be to mention in the comment that the Text.fromString referred to can be found in evancz/elm-graphics.

@jszmajda

This comment has been minimized.

Show comment
Hide comment
@jszmajda

jszmajda Jan 16, 2017

my error, thanks for setting me straight.

jszmajda commented Jan 16, 2017

my error, thanks for setting me straight.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Jan 16, 2017

Contributor

Oh, don't worry. No error/problem at all. But maybe let's close this issue here and open a fresh/clean one which just states that the documentation for Basics.fromString : a -> String mentions Text.fromString : String -> Text without telling the reader where to find Text.fromString (namely not within the same package here, instead in evancz/elm-graphics now).

Contributor

jvoigtlaender commented Jan 16, 2017

Oh, don't worry. No error/problem at all. But maybe let's close this issue here and open a fresh/clean one which just states that the documentation for Basics.fromString : a -> String mentions Text.fromString : String -> Text without telling the reader where to find Text.fromString (namely not within the same package here, instead in evancz/elm-graphics now).

@jszmajda

This comment has been minimized.

Show comment
Hide comment
@jszmajda

jszmajda Jan 16, 2017

Oh np, I think you fixed the documentation bug in https://github.com/elm-lang/core/issues/809, so definitely happy to close :) I've started a thread on the mailing list to discuss further.

Thanks!

jszmajda commented Jan 16, 2017

Oh np, I think you fixed the documentation bug in https://github.com/elm-lang/core/issues/809, so definitely happy to close :) I've started a thread on the mailing list to discuss further.

Thanks!

@jszmajda jszmajda closed this Jan 16, 2017

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Jan 16, 2017

Contributor

No, no, in #809 the bug is not fixed at all. I closed that PR because it wasn't a fix.

Contributor

jvoigtlaender commented Jan 16, 2017

No, no, in #809 the bug is not fixed at all. I closed that PR because it wasn't a fix.

@dgreensp

This comment has been minimized.

Show comment
Hide comment
@dgreensp

dgreensp Mar 16, 2017

The docs still reference the non-existent module Text, so re-open?

dgreensp commented Mar 16, 2017

The docs still reference the non-existent module Text, so re-open?

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Mar 16, 2017

Contributor

Re-open what?

Contributor

jvoigtlaender commented Mar 16, 2017

Re-open what?

@dgreensp

This comment has been minimized.

Show comment
Hide comment
@dgreensp

dgreensp Mar 16, 2017

@jvoigtlaender I'm agreeing with you that this issue should be re-opened, since the docs still make reference to the non-existent module Text at https://github.com/elm-lang/core/blob/master/src/Basics.elm#L472

dgreensp commented Mar 16, 2017

@jvoigtlaender I'm agreeing with you that this issue should be re-opened, since the docs still make reference to the non-existent module Text at https://github.com/elm-lang/core/blob/master/src/Basics.elm#L472

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Mar 16, 2017

Contributor

Actually, it would be better to open a fresh one with more focus, as I suggested above.

Contributor

jvoigtlaender commented Mar 16, 2017

Actually, it would be better to open a fresh one with more focus, as I suggested above.

@dgreensp

This comment has been minimized.

Show comment
Hide comment
@dgreensp

dgreensp Mar 16, 2017

Ok. FWIW, I think the initial issue description states the problem perfectly. The issue title seems to be a misprint and should be Basics.toString references missing module, but the description is accurate. There are different possible solutions to the problem; if it were me I would remove or rewrite the offending sentence completely. It seems odd to reference a graphics library in the docs for toString. The point (I think) is that the String returned matches how the value would be written in source code. It doesn't matter whether the String is rendered to graphics, rich text, DOM, or logged to the console.

dgreensp commented Mar 16, 2017

Ok. FWIW, I think the initial issue description states the problem perfectly. The issue title seems to be a misprint and should be Basics.toString references missing module, but the description is accurate. There are different possible solutions to the problem; if it were me I would remove or rewrite the offending sentence completely. It seems odd to reference a graphics library in the docs for toString. The point (I think) is that the String returned matches how the value would be written in source code. It doesn't matter whether the String is rendered to graphics, rich text, DOM, or logged to the console.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Mar 16, 2017

Contributor

The initial description is one thing. Subsequent comments lead astray and talk about functionality unrelated to what us actually at issue. Why don't you simply create a new issue?

Contributor

jvoigtlaender commented Mar 16, 2017

The initial description is one thing. Subsequent comments lead astray and talk about functionality unrelated to what us actually at issue. Why don't you simply create a new issue?

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