search

PoiScript / orgize

A Rust library for parsing org-mode files.
https://poiscript.github.io/orgize/
MIT License
278 stars 34 forks source link

FR: Make properties iteration order deterministic #12

Closed calmofthestorm closed 4 years ago

calmofthestorm commented 4 years ago

Currently, the properties API uses std::collections::HashMap, which is non-deterministic in iteration order. This means that parsing and then emitting an org file is not idempotent -- each time the file is parsed then written, the properties in each headline can change.

If you're using git, this leads to spurious diffs. If you prefer to have your properties in a certain order, then that will also be lost.

An easy "fix" would be to use https://docs.rs/indexmap/1.3.2/indexmap/map/struct.IndexMap.html which would maintain the order of the properties, though this would then mean either changing the API, copying to a hash map each time, or maintaining both a hash map and an indexmap (or just an order list).

I'm currently working around this by doing my own parsing of org files into a tree of headlines, and then only using Orgize to change them, or parse them but not write them out. This means that headlines never change unless I change them, in which case their property order is lost.

calmofthestorm commented 4 years ago

Looks like it is mostly a drop-in replacement, API compatibility aside. This branch is just for personal use -- I don't plan a PR based on it: https://github.com/calmofthestorm/orgize/tree/indexmap . All existing tests pass.

Only other issue would be maybe it would be good to only enable the serde features on indexmap if they are enabled on orgize.

calmofthestorm commented 4 years ago

Using that branch, Orgize will serialize all my org files faithfully (that is, if f(x) parses and then emits the string, f(f(x)) = f(x)) with only one exception: If the input string does not end in a newline, f(x) will also lack a newline, but f(f(x)) adds one. This difference seems irrelevant to me, and f(f(f(x))) = f(f(x)) for all my files.

PoiScript commented 4 years ago

Thanks for the suggestion. I have not heard of indexmap before, so it may takes a little more time for me to understand how it works.

PoiScript commented 4 years ago

Would you mind to open a pull request? I will consider to put it under a feature gate and disable it by default. Because I think it's only required in some special use cases.

calmofthestorm commented 4 years ago

This was fixed by #25 provided you compile with --features indexmap (disabled by default).