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

Inline imports #10

Merged
merged 2 commits into from
Dec 7, 2016
Merged

Inline imports #10

merged 2 commits into from
Dec 7, 2016

Conversation

stoeffel
Copy link
Owner

not sure if I'm happy with this yet, though. maybe it should have it's own prefix.
see example/Mock.elm

closes #7 #1

@stoeffel stoeffel self-assigned this Nov 28, 2016
@stoeffel
Copy link
Owner Author

cc @rtfeldman since this was your idea.

@stoeffel stoeffel removed their assignment Nov 29, 2016
@stoeffel
Copy link
Owner Author

not sure if I'm happy with this yet, though. maybe it should have it's own prefix.

I changed my mind. I think this is better than having a special prefix for imports. I also think it's better than the current implementation. It's more obvious what modules are used for the examples and it allows to use special dev-modules.

@stoeffel
Copy link
Owner Author

@eeue56 any thoughts about this?

@eeue56
Copy link
Collaborator

eeue56 commented Nov 29, 2016

@stoeffel I think something that would be extra helpful would be the ability to define variables too in a let binding

@stoeffel
Copy link
Owner Author

Will think about that, but I would open a new pr for that. Will create an issue for let binding.

@stoeffel
Copy link
Owner Author

I think something that would be extra helpful would be the ability to define variables too in a let binding

I opened an issue for that #11

@stoeffel
Copy link
Owner Author

I wonder if we should allow:

{-|
    >>> import MyModule exposing
    ...     ( foo
    ...    , bar
    ...    )
-}

@rtfeldman
Copy link
Collaborator

rtfeldman commented Nov 30, 2016 via email

@eeue56
Copy link
Collaborator

eeue56 commented Nov 30, 2016 via email

@stoeffel
Copy link
Owner Author

stoeffel commented Dec 1, 2016

@eeue56 you can already do:

{-|
    >>> indexOfLineWithEquals
    ...    <| Array.fromList
    ...    <| String.lines "a\nb\nc = d\nf = d\n"
    2
-}

see example here https://github.com/stoeffel/elm-doc-test/blob/master/example/Mock.elm#L59

@rtfeldman
Copy link
Collaborator

The ... is pretty noisy. 😬

I get the argument for >>> over > but this makes me want to re-advocate for >.

Compare:

{-|
    >>> indexOfLineWithEquals
    ...    <| Array.fromList
    ...    <| String.lines "a\nb\nc = d\nf = d\n"
    2
-}
{-|
    > indexOfLineWithEquals
    |      <| Array.fromList
    |      <| String.lines "a\nb\nc = d\nf = d\n"
    2
-}

@stoeffel
Copy link
Owner Author

stoeffel commented Dec 1, 2016

hmmm... will think about it. maybe we should do an other poll.

@stoeffel
Copy link
Owner Author

stoeffel commented Dec 1, 2016

it doesn't feel noisy to me though. it's how Python and elixir does it too

@stoeffel
Copy link
Owner Author

stoeffel commented Dec 1, 2016

I actually think | is more distracting. It looks like a broken sumtype or |>. Meaning, it resembles actual syntax more than ...

@rtfeldman
Copy link
Collaborator

Other alternatives:

{-|
    > indexOfLineWithEquals
    #      <| Array.fromList
    #      <| String.lines "a\nb\nc = d\nf = d\n"
    2
-}
{-|
    > indexOfLineWithEquals
    ~      <| Array.fromList
    ~      <| String.lines "a\nb\nc = d\nf = d\n"
    2
-}
{-|
    > indexOfLineWithEquals
    \      <| Array.fromList
    \      <| String.lines "a\nb\nc = d\nf = d\n"
    2
-}

@rtfeldman
Copy link
Collaborator

Also reminded of this because I saw @ento use the > style in an unrelated PR and I ❤️ how it's such clear documentation, it's what people choose to write even if there's no intention to generate tests!

@stoeffel stoeffel mentioned this pull request Dec 1, 2016
@stoeffel
Copy link
Owner Author

stoeffel commented Dec 1, 2016

@rtfeldman thanks for the input. I created an issue to discuss syntax changes there, because it's unrelated to wether or not we inline imports. #12

@eeue56 @rtfeldman are you okay with inlining imports or is anyone opposed?

@eeue56
Copy link
Collaborator

eeue56 commented Dec 1, 2016

I definitely think sticking with ... is the right syntax for multine things. when it comes to documentation, which doctests are all about, ... makes most visual sense.

seems good to me

@stoeffel
Copy link
Owner Author

stoeffel commented Dec 3, 2016

reminder for @stoeffel: merge this tomorrow and publish new version

@stoeffel stoeffel merged commit 35f70b3 into master Dec 7, 2016
@stoeffel stoeffel deleted the inline-imports branch December 7, 2016 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inline imports
3 participants