lineComment messes up newline tracking

[AlienKevin]

Parser Version

1.1.0

Description

Following the doc on specifying spaces for Elm:

sps : Parser ()
sps =
  loop 0 <| ifProgress <|
    oneOf
      [ lineComment "--"
      , multiComment "{-" "-}" Nestable
      , spaces
      ]

ifProgress : Parser a -> Int -> Parser (Step Int ())
ifProgress p offset =
  succeed identity
    |. p
    |= getOffset
    |> map (\newOffset -> if offset == newOffset then Done () else Loop newOffset)

The above works well until we introduce position tracking:

type alias Located a =
  { from : (Int, Int)
  , value : a
  , to : (Int, Int)
  }

located : Parser a -> Parser (Located a)
located p =
  succeed Located
    |= getPosition
    |= p
    |= getPosition

Now if we specify a simple test parser that uses the sps function:

parser : Parser (Located (), Located ())
parser =
    succeed (Tuple.pair)
        |= (located <| token "one")
        |. sps
        |= (located <| token "two")

and call parser one a simple source string:

source : String
source =
    """one -- comment
two
    """

result =
    run parser source

We unexpectedly got back:

Ok ({ from = (1,1), to = (1,4), value = () },{ from = (3,1), to = (3,4), value = () })

Notice that two is one the second line but elm/parser gives back its location on the third line:

{ from = (3,1), to = (3,4), value = () }

We are expecting:

Ok ({ from = (1,1), to = (1,4), value = () },{ from = (2,1), to = (2,4), value = () })

Workaround for lineComment:
Parser version:

lineCommentWorkAround : String -> Parser ()
lineCommentWorkAround start =
    succeed () |. symbol start |. chompWhile (\c -> c /= '\n')

Parser.Advanced version:

lineCommentWorkAround : Token -> Parser ()
lineCommentWorkAround start =
    succeed () |. symbol start |. chompWhile (\c -> c /= '\n')

Reason

This issue is likely because chompUntil and chompUntilEndOr does not consume the last string. lineComment is defined using chompUntilEndOr as follows:

lineComment : Token x -> Parser c x ()
lineComment start =
  ignorer (token start) (chompUntilEndOr "\n")

Because chompUntilEndOr updates the position but does not consume the last string, which in this case is \n, the newline is counted twice in the sps function. The second count results from the 3rd option spaces of the sps function which consumes the leftover newline at the end of the line comment. #20 has a PR that solves this bug but is still not merged. Can we review and merge it?

See and run the full test code including the workaround in Ellie.

Full source code for the test

module Main exposing (main)

import Browser
import Html exposing (Html, pre, text, h1)
import Html.Events exposing (onClick)
import Parser exposing (..)

type alias Model =
{ }

initialModel : Model
initialModel =
{ }

type Msg
= DoNothing

update : Msg -> Model -> Model
update msg model =
model

source1 : String
source1 =
"""one -- comment before newline
two -- comment before end of file"""

source2 : String
source2 =
"""one -- comment before newline
two - comment before end of file"""

view : Model -> Html Msg
view model =
pre []
[ h1 [] [ text "Normal source string without error" ]
, text <| "Source string:\n"
, text <| source1 ++ "\n\n"
, text "lineComment:\n"
, text <| (Debug.toString <| run parser source1) ++ "\n"
, text <| "Notice that two is one the 2nd line but lineComment gives back its location on the 3rd line" ++ "\n\n"
, text "lineCommentWorkAround:\n"
, text <| (Debug.toString <| run parserWorkAround source1) ++ "\n"
, text <| "lineCommentWorkAround gives back the correct location\nfor both comments before newline and before end of file"
, h1 [] [ text "misspelled '--'" ]
, text <| "Source string:\n"
, text <| source2 ++ "\n\n"
, text "lineComment\n"
, text <| (Debug.toString <| run parser source2) ++ "\n"
, text <| "Notice that the error happends at the end of the 2nd line but lineComment thnks it's on the 3rd line" ++ "\n\n"
, text "lineCommentWorkAround\n"
, text <| Debug.toString <| run parserWorkAround source2
, text <| "lineCommentWorkAround gives back the correct location"
]

parser : Parser (Located (), Located ())
parser =
succeed (Tuple.pair)
|= (located <| token "one")
|. sps
|= (located <| token "two")
|. sps
|. end

parserWorkAround : Parser (Located (), Located ())
parserWorkAround =
succeed (Tuple.pair)
|= (located <| token "one")
|. spsWorkAround
|= (located <| token "two")
|. sps
|. end

type alias Located a =
{ from : (Int, Int)
, value : a
, to : (Int, Int)
}

located : Parser a -> Parser (Located a)
located p =
succeed Located
|= getPosition
|= p
|= getPosition

sps : Parser ()
sps =
loop 0 <| ifProgress <|
oneOf
[ lineComment "--"
, multiComment "{-" "-}" Nestable
, spaces
]

spsWorkAround : Parser ()
spsWorkAround =
loop 0 <| ifProgress <|
oneOf
[ lineCommentWorkAround "--"
, multiComment "{-" "-}" Nestable
, spaces
]

lineCommentWorkAround : String -> Parser ()
lineCommentWorkAround start =
succeed () |. symbol start |. chompWhile (\c -> c /= '\n')

ifProgress : Parser a -> Int -> Parser (Step Int ())
ifProgress p offset =
succeed identity
|. p
|= getOffset
|> map (\newOffset -> if offset == newOffset then Done () else Loop newOffset)

main : Program () Model Msg
main =
Browser.sandbox
{ init = initialModel
, view = view
, update = update
}