WnP / vimwiki_markdown

vimwiki markdown file to html with syntax highlighting.
MIT License
63 stars 17 forks source link

Better VIMWIKI_MARKDOWN_EXTENSIONS environment variable parsing #18

Closed sak96 closed 3 years ago

sak96 commented 3 years ago
  1. Json list syntax of extension. ["toc", "nl2br"].
  2. Json dictionary syntax of extension with configuration {"toc": {"baselevel": 2 }, "nl2br": {}}. Note: {} configuration implies no configuration.
  3. [LEGACY] comma separated list of extensions toc,nl2br.
sak96 commented 3 years ago
let $VIMWIKI_MARKDOWN_EXTENSIONS="{'codehilite': {'linenums': True}, 'fenced_code': {}, 'tables' : {}}"

@WnP updated readme to explain the same.


I can do it in separate PR.

     if isinstance(extensions_env, list):
         extensions = extensions_env
-        extension_configs = {}
+        for extension in extensions_env:
+            if isinstance(extension, dict):
+                extension_configs.update(extension)
+            elif isinstance(extension, str):
+                extensions.append(extension)
+            # TODO: what do we do here ??
+        # TODO: is this really required ??
+        extensions.extend(extension_configs.keys())
     elif isinstance(extensions_env, dict):
         extensions = list(extensions_env.keys())
WnP commented 3 years ago

hi @sak96,

In fact I see it way simpler, something like the following should be enough:

import json

# parse extensions
extensions_env = os.getenv("VIMWIKI_MARKDOWN_EXTENSIONS", "{}")
try:
    extensions = json.loads(extensions_env)
except json.decoder.JSONDecodeError:
    extensions = extensions_env.split(',')

if isinstance(extensions, list):
    extensions = {e: {} for e in extensions}

# Setup markdown parser
md = markdown.Markdown(
    extensions=set(
        list(extensions.keys()) + ["fenced_code", "tables", "codehilite"]
    ),
    extension_configs=extensions,
)

IMHO, ast.iteral_eval is overkill and not safe. json sounds more appropriate here.

The colon separated list format should be deprecated and will be removed in future release.

No need for a separate pull request. Also please squash your commits.

Let me know your thoughts.

Thanks

sak96 commented 3 years ago

done

sak96 commented 3 years ago

done.

sak96 commented 3 years ago

need to recheck

sak96 commented 3 years ago

json load vs literal eval is

TLDR: "json syntax" != python_syntax

WnP commented 3 years ago

literal eval can parse single quoted strings

Yes, but look:

let $VIMWIKI_MARKDOWN_EXTENSIONS='{"codehilite": {"linenums": true}, "fenced_code": {}, "tables" : {}}'

vs

let $VIMWIKI_MARKDOWN_EXTENSIONS="{'codehilite': {'linenums': True}, 'fenced_code': {}, 'tables' : {}}"

... can't see where the issue is.

literal eval use python syntax (True instead of true of json)

Here we just need to inject some data / parameter / config, we do not need to eval whatever python code. That's why json is more appropriate.