Skip to content

Conversation

maranos
Copy link

@maranos maranos commented Aug 9, 2014

...ed to replace quotes. This makes it possible to use the correct quotes in languages other than English.

Use e.g. the following config for correct German quotes:

extensionConfigs = {
    'smarty': [('smart_lsquo', '‚'), # sb is not a typo!
               ('smart_rsquo', '‘'),
               ('smart_ldquo', '„'),
               ('smart_rdquo', '“')]
}

… used to replace quotes. This makes it possible to use the correct quotes in languages other than English.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 139e882 on maranos:master into 44cf8ce on waylan:master.

@mitya57
Copy link
Collaborator

mitya57 commented Aug 9, 2014

Please:

  • Use different names for configuration variables. They should not follow the same naming scheme as the existing ones, as it would lead to confusion. I suggest using [lr][sd]quo_replacement names.
  • Use consistent indentation in configuration variables description.
  • Add some tests.

@waylan
Copy link
Member

waylan commented Aug 10, 2014

I think @mitya57 has a point. The smart_* config keys are enabling/disabling features, whereas yours are changing the behavior of a feature. Using a different naming scheme might make sense ... Or, perhaps a dict could be passed in to one key. Something like chars={'lsqou': '‚'}. I'm unsure which would be better.

Also this definitely needs tests before consideration. Perhaps the example you added to the docs could be converted into a test. And while you're at it, if you could keep the docs hard wrapped at 80 chars, that would be preferred.

…h allows to overwrite all substitution strings. Fixed line length in docs.
@maranos
Copy link
Author

maranos commented Aug 11, 2014

Ok, I replaced the smart_lsquo etc. keys by a single one: smart_substitutions and I updated the docs accordingly. However, I don't understand how I can specify a dict as configuration option in the test.cfg file, so currently I cannot write a test.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) when pulling af85029 on maranos:master into 44cf8ce on waylan:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) when pulling 927ad00 on maranos:master into 44cf8ce on waylan:master.

@waylan
Copy link
Member

waylan commented Aug 12, 2014

You're correct that INI test config files cannot contain python dicts. There are a few other extensions which need more complex tests that the INI file can provide and their tests are contained in tests/test_extensions.py. You should include some tests there.

@maranos
Copy link
Author

maranos commented Aug 14, 2014

tests/test_extensions.py now has a test case for the new feature.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) when pulling e90687e on maranos:master into 44cf8ce on waylan:master.

@waylan
Copy link
Member

waylan commented Aug 15, 2014

@mitya57 I just gave you commit access. Smarty was your work. I'll let you merge this, and/or clean it up as you see fit.

@mitya57
Copy link
Collaborator

mitya57 commented Aug 15, 2014

Thanks, I will get to this in a couple of days.

And, actually, I still prefer that you do s/smart_substitutions/substitutions/ and s/Overwrite/overwrite/.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) when pulling e7b6a33 on maranos:master into 44cf8ce on waylan:master.

@mitya57 mitya57 merged commit e7b6a33 into Python-Markdown:master Aug 16, 2014
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.

5 participants