Skip to content
This repository has been archived by the owner on May 2, 2020. It is now read-only.

Optional markdown style configuration #41

Closed
wants to merge 1 commit into from
Closed

Optional markdown style configuration #41

wants to merge 1 commit into from

Conversation

peter1000
Copy link
Contributor

#39

I did not add any additional tests.

Optional `markdown` style configuration.
Stripped trailing spaces.
Added: `using Lexicon` to Example to make it selfcontained.
for (k, v) in mdstyle
!(k in reg_keys) && error("Invalid mdstyle key: `$k`. Valid keys: [$(join(reg_keys, ", "))].")
!(v in vaild_tags) && error("Invalid mdstyle value: `$v`. Valid values: [$(join(vaild_tags, ", "))].")
end
Copy link
Owner

Choose a reason for hiding this comment

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

The ! ... && style conditions may read better as ... || conditions. So:

length(keys(mdstyle)) == length(reg_keys) || error(...

(k in reg_keys) || error(...

(v in vaild_tags) || error(...

They should be logically equivalent I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change that.
I just started with julia and decided for the beginning to use only &&

Both && and || associate to the right, but && has higher precedence than || does (http://julia.readthedocs.org/en/latest/manual/control-flow/#short-circuit-evaluation)

@MichaelHatherly
Copy link
Owner

Great stuff! Left a couple of comments inline for discussion.

@peter1000
Copy link
Contributor Author

Thanks for the feedback: anyway my last push I have to test as I recomple julia.
I think there might be errors.

I will do that a bit later.

@peter1000 peter1000 closed this Apr 4, 2015
@peter1000 peter1000 deleted the adds_markdown_style branch April 4, 2015 15:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants