Fix other <|> breakage #164
Conversation
|
cc: @natefaubion is this how you would expect these operators to be changed? |
|
Oh hmm, 3 doesn't work because it's not an associative operator. |
|
I think ideally, to retain the same kind of formatting as before. It would need to be 4, but then it conflicts with precedence of functor operators. Unfortunately |
| @@ -132,13 +132,13 @@ infixr 2 withErrorMessage as <?> | |||
| withLazyErrorMessage :: forall m s a. Monad m => ParserT s m a -> (Unit -> String) -> ParserT s m a | |||
| withLazyErrorMessage p msg = p <|> defer \_ -> fail ("Expected " <> msg unit) | |||
|
|
|||
| infixl 3 withLazyErrorMessage as <~?> | |||
| infixr 3 withLazyErrorMessage as <~?> | |||
There was a problem hiding this comment.
Order of arguments means this has to be infixl.
| @@ -119,7 +119,7 @@ import Text.Parsing.Parser (ParseError(..), ParseState(..), ParserT(..), fail) | |||
| withErrorMessage :: forall m s a. Monad m => ParserT s m a -> String -> ParserT s m a | |||
| withErrorMessage p msg = p <|> fail ("Expected " <> msg) | |||
|
|
|||
| infixr 2 withErrorMessage as <?> | |||
| infixr 3 withErrorMessage as <?> | |||
There was a problem hiding this comment.
Order of arguments means this needs to be infixl.
|
Wow, operator precedence is hard. Nevermind, infixl things should be 4. This is fine and compatible with functor operators. |
|
Or to be super clear: infixl 4 withErrorMessage as <?>
infixl 4 withLazyErrorMessage as <~?>
infixr 3 asErrorMessage as <??> |
test/Main.purs
Outdated
| parseErrorTestMessage | ||
| (string "x" <|> "failure" <??> string " " <|> "bad" <??> string "-") | ||
| "no" | ||
| "Expected failure" |
There was a problem hiding this comment.
It might be helpful to have a <$> test in here just in case.
There was a problem hiding this comment.
I added <* in there, too, and then got a little carried away.
| @@ -22,7 +22,7 @@ jobs: | |||
| - name: Cache PureScript dependencies | |||
| uses: actions/cache@v2 | |||
| with: | |||
| key: ${{ runner.os }}-spago-${{ hashFiles('**/*.dhall') }} | |||
| key: ${{ runner.os }}-spago-${{ hashFiles('**/*.dhall') }}-2 | |||
There was a problem hiding this comment.
What's this change about?
There was a problem hiding this comment.
It busted CI cache, so that it would get the master version of control where <|> is right-associative. Without it, it uses the old cache where <|> is still left-associative.
|
@natefaubion Thoughts on the last formatting commit? |
|
Tidy doesn't have an updated operator table, unfortunately. You could potentially generate one yourself. |
test/Main.purs
Outdated
| skipMany1Rec (string "a") *> string "b" | ||
| parseTest "aaaabcd" "b" | ||
| $ skipMany1Rec (string "a") | ||
| *> string "b" |
|
Adding one doesn't seem to help much either... |
| Text.Parsing.Indent.(<?/>) 12 | ||
| Text.Parsing.Parser.Combinators.(<?>) 4 | ||
| Text.Parsing.Parser.Combinators.(<??>) 3 | ||
| Text.Parsing.Parser.Combinators.(<~?>) 4 |
There was a problem hiding this comment.
You need to add all spago sources.
There was a problem hiding this comment.
ie, the operator tables don't inherit from the default set.
There was a problem hiding this comment.
Huh... I ran purs-tidy generate-operators "$(spago sources)" > .tidyoperators to get the above output.
After reading the above comment, I then tried what your readme says:
spago sources | xargs purs-tidy generate-operators > .tidyoperators
I still get the same thing.
There was a problem hiding this comment.
Maybe xargs behaves differently on your system? I just tried it and I get the full prelude in there.
There was a problem hiding this comment.
This is what I got https://gist.github.com/natefaubion/5fadb674d643666272aa07959559880e
Description of the change
Follow up to #163. Relates to #161.
Checklist: