Fix other <|> breakage#164
Fix other <|> breakage#164JordanMartinez merged 17 commits intopurescript-contrib:mainfrom JordanMartinez:fix-alt-stuff
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 |
| 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.
| 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.
| 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.
3 participants
Description of the change
Follow up to #163. Relates to #161.
Checklist: