-
Notifications
You must be signed in to change notification settings - Fork 136
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
add support for [\n] character format parameter inside config file #62
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.
Overall looks good, just some minor things.
(Although I don't actually have any authority in this project)
types.c
Outdated
char ch; | ||
while ((ch = *location++) != '\0') { | ||
switch (state) { | ||
case MAKO_PARSE_STATE_ESCAPE: |
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 think the style is to not indent case labels.
types.c
Outdated
@@ -123,3 +124,55 @@ bool parse_directional(const char *string, struct mako_directional *out) { | |||
free(components); | |||
return true; | |||
} | |||
|
|||
bool parse_format(const char *string, char **out) { | |||
int token_max_length = strlen(string) + 1; |
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.
size_t is more correct here.
types.c
Outdated
const char *location = string; | ||
|
||
char ch; | ||
while ((ch = *location++) != '\0') { |
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 think a normal for loop would be better.
types.c
Outdated
break; | ||
|
||
case '\\': | ||
default: |
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 sure if you want to ignore unknown escape sequences or not. Sometimes that can mess with the config file's forwards compatibility.
That's up to @emersion though.
Thanks for your PR! I agree with @ascent12's comments (thanks!).
I'm not super-concerned about this. Maybe keeping unknown escaped sequences as-is would be a little bit better. |
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 seems that I just completely forgot about this. Please ping me next time!
Thanks! |
sorry, i should have pinged you. |
This fixes #61
As a consequence also the char
\
needs to be escaped.i'll update the
mako.1.sdc
file once the modification are approved