Fix infix constructor pattern matching for normal constructors #107

2018-01-15T22:04:31+01:00

eborden commented

2018-01-15 22:04:31 +01:00

(Migrated from github.com)

Brittany was previously only supporting symbol based infix constructors. It
is common in some libraries (for example Esqueleto) to pattern match on
normal constructors as infix. Brittany was failing in this case by not
wrapping the constructor name in back ticks/spaces. Backticks and spaces
have been added in the case where the constructor contains any alpha
characters.

This addresses #101

Brittany was previously only supporting symbol based infix constructors. It is common in some libraries (for example Esqueleto) to pattern match on normal constructors as infix. Brittany was failing in this case by not wrapping the constructor name in back ticks/spaces. Backticks and spaces have been added in the case where the constructor contains any alpha characters. This addresses #101

eborden commented

2018-01-15 22:07:00 +01:00

(Migrated from github.com)

This could be moved to the dev branch, but it seems like a simple enough fix to make it in to master via a point release.

This could be moved to the dev branch, but it seems like a simple enough fix to make it in to `master` via a point release.

lspitzner commented

2018-01-16 00:33:04 +01:00

(Migrated from github.com)

Thanks! I think using lrdrNameToTextAnn instead of lrdrNameToText is a better solution, as it does not depend on isAlpha. And the spaces should be added unconditionally, to be consistent with operator space handling in other cases.

Please change to dev. I agree with making a minor release for this, but the other stuff currently on dev deserves to be included, too :)

Thanks! I think using `lrdrNameToTextAnn` instead of `lrdrNameToText` is a better solution, as it does not depend on `isAlpha`. And the spaces should be added unconditionally, to be consistent with operator space handling in other cases. Please change to `dev`. I agree with making a minor release for this, but the other stuff currently on `dev` deserves to be included, too :)

eborden commented

2018-01-16 01:26:00 +01:00

(Migrated from github.com)

Hmm, getting some strange business with trying to rebase off upstream. Seems some commits have snuck in.

Hmm, getting some strange business with trying to rebase off `upstream`. Seems some commits have snuck in.

eborden commented

2018-01-16 16:04:13 +01:00

(Migrated from github.com)

Looks like OSX made it through the tests, but then timed out.

eborden commented

2018-02-05 16:48:51 +01:00

(Migrated from github.com)

@lspitzner do you see any blockers on this PR?

lspitzner commented

2018-02-13 22:56:50 +01:00

(Migrated from github.com)

Sorry for the long delay. I have effectively merged this PR now, although I think github won't see this because I rebased/cherry-picked again. But the commits are now on dev. I am working on a fix for #116 and will do a release once that is done.

(And you are right, the test failure is irrelevant.)

Sorry for the long delay. I have effectively merged this PR now, although I think github won't see this because I rebased/cherry-picked again. But the commits are now on `dev`. I am working on a fix for #116 and will do a release once that is done. (And you are right, the test failure is irrelevant.)

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

No reviewers

No Milestone

No project

No Assignees

1 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: hexagoxel/brittany#107