-
Notifications
You must be signed in to change notification settings - Fork 1
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
Implement validation #3
Conversation
frosch123
commented
Apr 23, 2024
- Usage is explained in the readme: https://github.com/frosch123/nile-validator/blob/wip/README.md
- I use the term "dialect" for what nile-config calls "format". "format" sounded like "txt" vs. "json" to me. The values for "dialect" use the same spelling/capitalisation as Bananas.
- There are some bells and whistles left, but otherwise this is complete.
- The commit history is mostly chaos, so squash it is.
} | ||
|
||
/// Replace all ASCII control codes with blank. | ||
/// Remove trailing blanks at end of each line. |
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.
Is this meant as doxygen-style comment? Or a reminder to yourself?
Otherwise, can I suggest /** .. */
instead?
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.
This is meant as rustdoc
, which seems to use ///
.
struct StringSignature { | ||
parameters: HashMap<usize, &'static CommandInfo<'static>>, | ||
nonpositional_count: BTreeMap<String, (Occurence, usize)>, | ||
// TODO track color/lineno/colorstack for positional parameters |
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.
still todo?
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.
Yes, this is part of the "bells and whistles". Currently stashed.
config: &LanguageConfig, | ||
test: &ParsedString, | ||
base: Option<&ParsedString>, | ||
) -> Vec<ValidationError> { |
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.
This function is over 300 lines long; any chance we can split it up a bit?
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.
Yes, I am aware. There is also some code duplication with other methods. I need some kind of generator/iterator to loop over commands and figureing out positional parameter references.
Also part of "bells and whistles"