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

Some function types generate parsing warning in markdown mode #15

Open
jwoLondon opened this issue Dec 13, 2018 · 11 comments
Open

Some function types generate parsing warning in markdown mode #15

jwoLondon opened this issue Dec 13, 2018 · 11 comments
Labels
bug Something isn't working priority/p2

Comments

@jwoLondon
Copy link
Member

For the record, we still need to implement markdown output for a wider range of types.

Currently some types can generate parsing warnings. For example

```elm {m}
myFunction : List ( Int, Int )
myFunction =
    [ ( 1, 2 ) ]
```

generates the warning : Could not parse "[(1,2)]" (litvis:expression-value) highlighting the entire code block.

We should also test against those examples listed in #13 and #14.

If this isn't fixable in the short term, we should probably generate a more beginner friendly and correct warning (the problem isn't with the contents of the function, but the output type of the code block) such as Markdown output not supported for this type. Could you use 'raw' (or 'r') output instead?

@jwoLondon jwoLondon added bug Something isn't working priority/p2 labels Dec 13, 2018
@kachkaev
Copy link
Member

This issue with tuples will likely apply to j and v outputs too. It seems that elm-string-representation struggles with turning [(, ),( and )] into a valid JSON.

What would be a representation of tuples in JSON?

[(1,2)]
[(1,2),(3,4)] # list of tuples

[[1,2]]
[[1,2],[3,4]]

?

@jwoLondon
Copy link
Member Author

If I have understood correctly, we need a JSON representation that distinguishes a tuple (x,y) from a list with two (or three) elements [x,y] both of which are valid in Elm but distinct.

If this is the case we cannot use simple square brackets as a list of lists is also valid elm ([[x,y]]). How about representing the tuple with a JSON object

// Single tuple
{
"openTuple" : "(",
"content" : [1,2],
"closeTuple" : ")"
}


// List of tuples
[ { "openTuple" : "(",
  "content" : [1,2],
  "closeTuple" : ")"
  },
  { "openTuple" : "(",
   "content" : [3,4],
   "closeTuple" : ")"
  }
]

@kachkaev
Copy link
Member

If we want our transformation to be revertible (i.e. original == stringify(parse(original))), then representing tuples in some special form would be a requirement. Otherwise, just an array will probably work. In Elm, when I send tuples over a port or want to serialize them into a JSON, what are they converted to?

@jwoLondon
Copy link
Member Author

To send anything over a port it has to be explicitly serialised into a JSON by the programmer. The Json.Encode package shows what is available (string, int, float, bool, null, list, array and object). Note that the tuple is not one of these (quite possibly why we have this issue). So we will need to encode ourselves as one of those types. So as you say, assuming we never need to revert, creating a list makes sense.

@kachkaev
Copy link
Member

Let's stick with converting (1,2) to [1,2] for now.

Later on, we'll be able to enhance the parse() function by adding the second optional argument. It will either contain some flag that would trigger a different behaviour for tuples or even a mapper function, which would allow users to define what structure they want their tuples to be converted to.

@kachkaev
Copy link
Member

Will try resolving this in the evening.

@jwoLondon
Copy link
Member Author

There are still a few cases that are causing linter warnings. Specifically tuples containing Bool types and parameterised functions (which I think is a regression as this used to work). MWEs:

```elm {m}
boolTupleTest : ( Bool, Bool )
boolTupleTest =
    ( False, True )
```
```elm {m}
functionTest : Int -> Int
functionTest n =
    n + 1
```

BTW, I spotted this when running this test, which might be a useful one to add to the test suite:

```elm {l m}
type alias MyRecord =
    { intItem : Int
    , strItem : String
    , booItem : Bool
    , lstItem : List Int
    , tplItem : ( Int, Int )
    , tp3Item : ( Bool, Int, String )
    , tLsItem : List ( Float, Int )
    , rcdItem : { nestedName : String, nestedValue : Int }
    , fnItem : Int -> Int
    }
```

```elm {l m}
recordTest : MyRecord
recordTest =
    MyRecord 1 "two" True [ 3, 4, 5 ] ( 6, 7 ) ( False, 8, "nine" ) [ ( 1.0, 11 ), ( 1.2, 13 ) ] { nestedName = "fourteen", nestedValue = 15 } (\n -> n + 1)
```

@jwoLondon jwoLondon reopened this Jan 7, 2019
@kachkaev
Copy link
Member

Thanks for digging these out Jo. I'll add the above cases to tests during the next litvis dev session.

@kachkaev
Copy link
Member

I'll look at this now.

@kachkaev
Copy link
Member

Partially applied functions are serialized to "<function>" (JSON strings).

@jwoLondon
Copy link
Member Author

Markdown output does not yet parse Char types (generates a parsing error in litvis). I recommend we treat these just as we do Strings, i.e. without quotation marks around them, space separated when in a list or tuple.

We can add the following to the tests:

```elm {m}
charsTest : List Char
charsTest =
    [ 'a', 'b', 'c' ]
```

```elm {m}
charTupleTest : ( Char, Char )
charTupleTest =
    ( 'a', 'b' )
```

And modify the record test:

```elm {l m}
type alias MyRecord =
    { intItem : Int
    , strItem : String
    , booItem : Bool
    , charItem : Char
    , charItems : List Char
    , charTuple : ( Char, Char )
    , lstItem : List Int
    , tplItem : ( Int, Int )
    , tp3Item : ( Bool, Int, String )
    , tLsItem : List ( Float, Int )
    , rcdItem : { nestedName : String, nestedValue : Int }
    , fnItem : Int -> Int
    }
```

```elm {l m}
recordTest : MyRecord
recordTest =
    MyRecord 1 "two" True 'X' [ 'a', 'b', 'c' ] ( 'd', 'e' ) [ 3, 4, 5 ] ( 6, 7 ) ( False, 8, "nine" ) [ ( 1.0, 11 ), ( 1.2, 13 ) ] { nestedName = "fourteen", nestedValue = 15 } (\n -> n + 1)
```

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority/p2
Projects
None yet
Development

No branches or pull requests

2 participants