Move opts to own map

[mk]

No description provided.

[mk]

Not sure about needing more changes in parser.

[borkdude]

@mk I considered doing this, but the reason I didn't proceed, was that "dissoc option from empty-doc to disable option" was a contract from the beginning. Killing that contract will be a breaking change.

markdown/src/nextjournal/markdown/impl/utils.cljc

Line 55 in 6c0df00

;; Node -> {id : String, emoji String}, dissoc from context to opt-out of ids

[borkdude]

@mk @zampino More thoughts:

• We could check if parse* is called with a document by checking the essential keys :type, :content, ::root. If that is the case, and :text-tokenizers or one of the other two options :disable- and ...emoji-fn is missing, then that counts as disabling text tokenizers, etc.
• :opts should always override the implicit document options, but how do people disable :emoji-fn, but providing nil? Previously disabling the emoji fn was done by dissoc-ing it from the passed document.
• I realize now that my check here is wrong:

markdown/src/nextjournal/markdown/impl.clj

Lines 286 to 288 in 6c0df00

	(if (not (set/superset?
	(set (keys ctx))
	(set (keys u/empty-doc))))

I should've checked only for essential document keys and not count the implicit option keys since those can be dissoc-ed.

• I guess dissoc-ing the emoji-fn to opt out also currently doesn't work anymore with parse so that's already a breaking change?

We can also say: screw all of the above backward compatibility changes and we just put a big BREAKING in the changelog.

[zampino]

we just put a big BREAKING in the changelog.

I'm all for that. Did we find usages of customization of the emoji/id extractor?

[borkdude]

I should've checked only for essential document keys and not count the implicit option keys since those can be dissoc-ed.

This is now fixed in PR #62

[borkdude]

I'm all for that. Did we find usages of customization of the emoji/id extractor?

I'd say no: https://github.com/search?q=%22text-%3Eid%2Bemoji-fn%22&type=code

[mk]

was that "dissoc option from empty-doc to disable option" was a contract from the beginning. Killing that contract will be a breaking change.

Since this wasn't documented, I'd be surprised if folks are using this already. I'd vote for making the api better long term and go ahead with this minor breaking change.

[borkdude]

Since this wasn't documented, I'd be surprised if folks are using this already

Fair point

[borkdude]

@mk Except if you consider this notebook as documentation:

4f628e1

I updated the notebook now though.