Tidying of the C/C++/ObjC code #237
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Requires a bit of header tidying because some of the XeTeX stringpool functions have the same names as ones in bibtex.c. This tidying is good since we should be clearer about partitioning the namespaces anyway.
And tidy up related symbols. These files and functions were badly named because they are specific to the XeTeX engine -- they don't have anything to do with bibtex or dvipdfmx.
Once again, these functions are specific to the XeTeX engine.
This is intended to be the header that does the basic grunt work of defining foundational types and setting up basic portability matters. At the moment it's just stealing definitions from tectonic.h, but I'll move more things here.
Pulling more functions out of internals.h, which I'd like to get rid of since its name is not super helpful and it currently contains a big mish-mash of functionality.
It's only used inside one function in xetex0.c, and a helper function that assists it.
This allows us to make several functions static and remove some types from the global headers.
It was in core-bridge.h, but this is a better place for it.
And move its prototypes to continue the process of emptying out "internals.h"
And add xetex-io.h, stealing a lot of definitions from "internals.h". Once again, these routines are specific to the XeTeX engine.
Honestly this isn't a way better location, but it's at least a *bit* better than "internals.h". And I'm trying to nuke the latter entirely.
The 0.1.10 Tectonic package on Conda fails to build on Windows because it is undefined. There weren't any problems on Tectonic's CI, though, so it must be version-dependent.
The last bit in "internals.h" was some dynamic memory routines. Break those out into their own files. Now "internals.h" is basicaly superfluous.
Have every xdvipdfmx header include a new "dpx-core.h" before doing anything else. At the moment the new "dpx-core.h" just includes "core-foundation.h", but I might add additional items that are essentially universal in the xdvipdfmx codebase. The main thing is to ensure that "core-foundatiion.h" is included before any other headers.
By adding the core #includes to "dpx-core.h", it is no longer needed.
As in xdvipdfmx, the build is preserved if we add a few core includes to xetex-core.h
Removed the last of the references to this poorly-conceived header.
At first I wasn't sure whether to fold these files into the `xetex-` prefix or not, but they really do form an isolated subsystem. There was also some confusion because there are essentially two Engine.h files -- one pure C, one C++.
This ended up not containing anything useful.
Final file names are along the lines of "xetex-XeTeXFontMgr.h" Note that the classes that these files define have names such as `XeTeXFontMgr`, so it seems best to keep the "XeTeX" in the filename.
Codecov Report
@@ Coverage Diff @@
## master #237 +/- ##
==========================================
- Coverage 42.13% 38.09% -4.05%
==========================================
Files 132 133 +1
Lines 63487 63994 +507
==========================================
- Hits 26753 24380 -2373
- Misses 36734 39614 +2880
Continue to review full report at Codecov.
|
|
I don't know why codecov thinks the coverage went down nontrivially, but that's got to be spurious. Away we go! |
Mrmaxmeier
pushed a commit
to Mrmaxmeier/tectonic
that referenced
this pull request
Dec 13, 2019
Use pub(crate) if possible.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Virtually no (intended) changes to the code, but here we organize the naming of the files and try to establish a better pattern of the various C/C++ headers. Old headers with names like "internals.h" and "tectonic.h" had a mish-mash of items specific to the XeTeX engine and items applicable to the overall C library. I think we do better now.
This PR renames a bunch of files, so things like coverage stats and any prior PRs to the C code are going to be more or less wiped out. Apologies if there's any work in progress that will be affected by these renames (but I'm not aware of any).