-
Notifications
You must be signed in to change notification settings - Fork 2
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
Refactor CLI and docs #153
Refactor CLI and docs #153
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, I would prefer if we keep the unpopular extensions or those that just provide (unintuitive) gimmicks to a minimum 🙂
, single :: Bool | ||
, json :: Bool | ||
, inputFile :: Maybe FilePath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you make it a named parameter instead of positional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to support reading a program:
- from a file:
--input-file program.phi
- from an option:
--program "{ ⟦ ⟧ }"
- from stdin:
--input-file -
So, I read from stdin only in the case 3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't agree with these. I think what we already had is more idiomatic and we already supported all three cases:
normalizer transform program.phi
echo "{ ⟦ ⟧ }" | normalizer transform
normalizer transform
This is what I think is the approach followed by most programs I used, and what we already supported
eo-phi-normalizer/app/Main.hs
Outdated
pprefs :: ParserPrefs | ||
pprefs = prefs (showHelpOnEmpty <> showHelpOnError) | ||
|
||
type Context = (?parserContext :: Optparse.Context) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with many extensions, what does this syntax mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's ImplicitParams.
I use this extension instead of the ReaderT pattern.
eo-phi-normalizer/app/Main.hs
Outdated
StructuredJSON | ||
{ input = printTree program' | ||
, output = (printAsProgramOrAsObject <$>) <$> results | ||
} | ||
else do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too much nesting of if
s leads to higher cognitive load when reading the code. Consider refactoring somehow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't think 1 nested if
is too much.
- `Dataless formation` - not primitive and does not have attributes bound to primitive formations. | ||
- `⟦ d ↦ ⟦ φ ↦ ξ.ρ.c, ν ↦ ⟦ Δ ⤍ 00- ⟧ ⟧, c ↦ ∅ ⟧` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So just to clarify, we mean it does not have immediate attributes bound to primitive formations, but it's ok if its descendants do?
So the formation bound to d
would be neither primitive nor dataless, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great if you can add this clarification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A formation doesn't have non-immediate attributes, does it?
…-metrics-for-a-given-phi-expression
CLI
defaultMain
until we want to use pre-defined rulesDocs
.md
filesPR-Codex overview
This PR updates the
normalizer
executable in the Haskell project. It adds new commands, modifies dependencies, and enhances documentation.Detailed summary
normalizer
executable.optparse-applicative
andaeson-pretty
.