Get rid of all global variables

[tshikaboom]

No description provided.

[blu-base]

Could you give me an example? Do you mean the variables declared in config.h?

[tshikaboom]

The config.h file is generated by the configure stage, so it can be completely ignored.

There are three extern declarations in the codebase:

• Two std::unordered_map<uint32_t, libone::GUID> GlobalIdentificationTable in libone_types.h and ObjectGroup.h (I think the second declaration is redundant anyway, although it's been some time I've been digging in the code...)
• libone::ExtendedGUID DataSignatureGroup also in libone_types.h

Both of these get instantiated and referred to by others in OneNoteParser.cpp. There are thankfully not many users of these at the moment.

The idea would be to instantiate something function-local in OneNoteParser::parse() (or maybe OneDocument::parse()) and pass this to functions that would be called afterwards. This can also be an opportunity to fold in the *input pointer defined as an argument everywhere and refer to it from the "super-structure" that would represent the current state of what we've parsed in the document.

Does that make any sense? :)

[blu-base]

Does that make any sense? :)
Yes, i think, I understand.

I am at a similar point with my library attempt, too.

I was just training design patterns, and thought a factory might be needed here, e.g. reading DocumentFactory().getCurrentDocument().getGlobalIdentifcationTable(). I played with a signleton here, to get "upstream" info from the objectgroups. But this left me stranded a bit, since a singleton would limit the library to only parsing one document at a time, or having only having one document active at a time. And, I didn't want to pass the document identifier (such as its name) down to every object.

I haven't looked much on that idea, but can have a librevenge stream meta information?
If it wouldn't, i would assume that a new stream class with additional meta info would be required. But this seems to be not an intelligent way...

although it's been some time I've been digging in the code...

yeah, you warned me, that you are currently very busy. Don't feel pressured to invest time.
Though i'll write down thoughts anyway, before i forget them... I also haven't compared all the code yet.

[tshikaboom]

Indeed, a singleton won't work because of the inherent limit of managing only one document. (as an aside, that might just work for issue #3, as logging would be done in one manner library-wide anyway).

I'm wondering though what you have in mind with the factory approach? If you have any more thoughts on this or anything. This might or might not work, so far I've avoided design patterns to keep things simple, but you might be onto something here.

Regarding stream properties, one way of appending metadata "private" to libone could be to use librevenge document properties if you want to limit yourself to what's provided by librevenge (one could prefix private data with libone_ or something), although I haven't tested the feasibility of this in practice. Even so, I think there's no way around doing a (big) structure private to libone itself if we want more control over what's happening, before using librevenge's document methods to actually render the document.
There might be information in the source code of librevenge, classes such as RVNGFileStreamPrivate and RVNGDirectoryStream might be helpful. Even if the former is supposed to stay private to the library, I found references to a member of type FILE * in there.
A much more simpler approach at the moment would be to just try only parsing the file we're asked to, without taking in account anything else (such as the other files), and we'll see as we go on. This would leave the complexities of managing multiple documents for afterwards. This still requires us to keep some stateful structure somewhere...

(this is a bit of a braindump, again..)