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

unitb-config revamp #34

Merged
merged 7 commits into from
Aug 24, 2017
Merged

unitb-config revamp #34

merged 7 commits into from
Aug 24, 2017

Conversation

bandali0
Copy link
Member

A series of changes to the literate-unitb-config executable, unitb-config:

  1. Swap out ConfigFile in favour of yaml for the configuration file. The configuration file was renamed from z3_config.conf to z3_config.yml to reflect the change.
  2. Generalize functions of the Document type class to reduce duplication (with respect to field, fieldWith, and their primed variants) and to accommodate for introduction of yaml.
  3. Rename the executable from unitb-option to unitb-config consistently.

Config file's name changed from z3_config.conf to z3_config.yml.

TODO: there's a slight issue with the yaml library treating all
of Z3Config's fields as String, in that it will require the numbers
to be explicitly quoted to be considered as string literals; and
otherwise it will ignore them. We'll try to fix this by generalizing
the Document type class and the functions using it.
- literate-unitb-config compiles again
- Add traverseOf (similar to lensOf and prismOf)
- Swap `fieldWith`'s arguments to make them consistent with `field`
yaml < 0.8.22 causes literate-unitb-logic's test suite to throw an
exception: Control.Concurrent.STM.atomically was nested

Possibly related: snoyberg/yaml#86

Not sure about the exact cause, but we don't seem to have the issue
with yaml >= 0.8.22, so I'm bumping the lower bound as a workaround.
-> BiParser Maybe b doc String
field :: (Document doc,ToJSON a,FromJSON a)
=> String -> (b -> a)
-> BiParser Maybe b doc a
field k f = BiParser (makeNode k . f) (lookupDoc k)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please align => and -> with ::?

-> Traversal' s a
traverseOf def (BiParser f g) h x =
(\i -> f i def) <$> h (runIdentity $ g x)
-- f <$> h (runIdentity $ g x) <*> pure def
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove this commented line please?

@@ -74,4 +74,4 @@ main = execParser opts >>= apply
where
opts = info ( helper <*> commandLineOpts )
$ fullDesc
<> header "unitb-setting - customize the preferences for verifier"
<> header "unitb-config - customize the preferences for verifier"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message was broken. Good thing you had to change it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah :)

@@ -21,6 +23,7 @@ instance (Applicative f) => Applicative (BiParser f a b) where

-- instance Profunctor (BiParser a) where
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this commented code. If we need a Profunctor instance, we'll just add it from scratch

@bandali0 bandali0 self-assigned this Aug 23, 2017
@cipher1024 cipher1024 merged commit 825bec0 into master Aug 24, 2017
@bandali0 bandali0 removed the request for review from cipher1024 August 24, 2017 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants