-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Swift Package Manager Support #200
Comments
To clarify:
Until there is a compelling reason to switch to Swift from C, I don't anticipate digging too far into this myself. If you use Swift, then it's fine to use the fork by @slsrepo, or if there is some consistent opinion from several users as to how to make it easier to use the core repo with Swift that does not require much ongoing maintenance, I am happy to merge some changes back into the main repo. Thanks, @slsrepo and others!! |
Addendum -- another reason NOT to use token pools is if you need access to the token tree for an indefinite period of time after the parsing was done. Which I do in Composer and nvUltra. |
Thanks Fletcher for the clarification :) These changes are indeed not related to Swift, but I'll clarify why I made them in the fork. The Swift Package Manager has five rules, requiring:
And while Swift supports custom C/Swift/Flags, Xcode doesn't support packages that require these. Since disabling the token pool requires either commenting the line or using Since the repo used to require the flag The last change is just including files like If anyone wants to enable the token pool, it's easy to just download the archive of the repo, uncomment the line in token.h and then use the package locally (using the local file path instead of the git repo URL) or in Xcode by dragging either the folder or the "Package.swift" file into your project. Another advantage I tested but not mentioned in the original post, is that Swift doesn't care about the architecture, it can build the library for any OS or architecture supported by Swift out of the box, including iOS, Mac (both Intel (x86_64, x86_64h) and ARM), Linux and Windows. And as I mentioned, as soon as I (or anyone else) can make CMake make the required changes automatically, it will require zero maintenance from your side, just running Hope others can share their thoughts too, maybe Christian (@DivineDominion) can contribute some thoughts :) |
I applaud the effort to make a SwiftPM package from this! I think that simplifies inclusion and also can help with adoption of this library, which I'd like to see. The changes should of course be minimal to not change how MMD6 works. It sounds like the token pool should be enabled in a run-once scenario, but disabled for long-lived usage in syntax highlighting. I think multiple package targets could do the trick here to disambiguate. (We've been tinkering with a fork with minimal changes to make the build process work for us in Xcode 12 late last year but before 6.6 was released. It's still experimental, and may be obsolete already.) |
Thank you for your efforts, it would be great to be able to simply include this as a Swift module. I added the package via Swift Packages tab in Xcode, then linked both MultiMarkdownSPM and libMultiMarkdown (at first I didn't include the latter, because it sounded like I only need it if I want to use the C symbols, but then I got an error and tried including it as well). Now when compiling in Xcode I simply get the error Is there something I can provide to help you understand what's going on? |
There's not much more:
And yes, I'm using version 6.6.0. |
I think it was an Xcode problem, sorry for the trouble. After closing the project and trying again, it worked. It also compiled successfully when trying the same steps on a copy of the original project in which I'm still using the Objective-C imports. Fantastic, it works! But I noticed two strange things:
This actually happened with both copies of my project I just mentioned, but only on the first run (right when launching the app, at which point the framework was not even being used).
|
I'm glad it works :) I assume it also means issue #199 is solved too? I'm almost sure these errors are not related to the package, or to the regular CMake built MultiMarkdown. It seems more like your Mac is having a networking issue (mDNSResponder is the main process used by the system to resolve DNS queries when you make any sort of network request, including HTTP). |
Well, #199 is solved for me now, but it may still be relevant for someone who wants to use the framework in an Objective-C project. Is there something I could to to rule out that the cause is the Multimarkdown framework? Because when commenting out the only line of code in my app that uses the framework ( |
You could place a breakpoint before and after the line, try downloading the forked repo and running the package locally (drag either the folder or the Package.swift file into your project and link the the libraries to the project like I showed in the screenshot above). You can also try cleaning Xcode's cache by clicking File > Swift Packages > Reset Packages Cache and Product > Clean Build Folder. But I can't see how it's related - neither the Swift Package or Fletcher's library use any sort of network request. |
Oh, how embarrassing. You're right, setting a breakpoint was the obvious thing to do, and it showed that after that line the console is still empty. So it's probably something that Xcode or macOS does with the HTML string. Thank you. By the way, it would be nice to mention in the readme what to import and what calls you can do with the framework, so that one doesn't have to rely on autocompletion to find out what to do. |
Another thing I just noticed, Have a look at your /// Convert a source String containing MultiMarkdown text to the requested `output_format`, using the provided `parser_extensions` (defaults are .Footnotes, .Smart, .CreateSnippet) and language for localization and smart quotes.
///
/// * Please note: This works best formats that work with single files (like HTML, LaTeX, etc). For complex formats that use multiple files (like EPub, TextBundle, OpenDocumentText, iThoughtsITMZ or MMD with Transclusion), you should use the .export function instead.
///
/// - parameters:
/// - source: `String` containing MultiMarkdown text.
/// - extensions: Requested `parser_extensions`.
/// - format: Requested `output_format`.
/// - language: Requested language for localization and smart quotes.
/// - returns: `String`, or `nil` if there was any issue.
public class func parse(_ source: String, extensions: Extensions = [.Footnotes, .Smart, .CreateSnippet], format: OutputFormat = .HTML, language: LocalizationLanguage = .English) -> String? {
// Convert MMD text to specified format, with specified extensions, and language
// Returned char * must be freed
let output = d_string_new("")
let e = mmd_engine_create_with_string(source, extensions.rawValue)
mmd_engine_set_language(e, language.rawValue)
mmd_engine_parse_string(e)
mmd_engine_export_token_tree(output, e, format.rawValue)
guard let resultPointer = output?.pointee else {
mmd_engine_free(e, true)
return nil
}
let result = String(cString: resultPointer.str)
mmd_engine_free(e, true) // The engine has a private copy of source that must be freed
d_string_free(output, false)
return result
} |
(I also get the same console output when using the Objective-C calls.) |
I can definitely add that to the README :) Glad you got it resolved :) (Funny how we posted in the same second :P) |
Cool :) Just to clarify, that comment above (detailing how |
I have a better idea for the token pool - We can make the definition of the token pool subject to another directive, something like this: #ifdef MMD_SHOULD_ENABLE_TOKEN_POOL:
#define kUseObjectPool 1
#endif Then, if someone defines MMD_SHOULD_ENABLE_TOKEN_POOL as a preprocessor directive for their project in Xcode, kUseObjectPool will be defined and the token pool will be enabled, while keeping it disabled for others. What do you think? :) |
@slsrepo The current version disables the token pool by default |
@DivineDominion Yes, you are right. I haven't pushed the change mentioned above because I was waiting for comments. But I have it ready to go. I'll try to push it later today :) |
@DivineDominion Done. You can give it a try now :) |
@slsrepo I opened an issue on the official repository a month ago, but it wasn't answered. Are you still maintaining it? |
Hi Fletcher, Everyone.
I've been working with MultiMarkdown since 2016, constantly talking to Fletcher via email.
Since Fletcher doesn't actively use Swift, I eventually created a fork of MultiMarkdown, compatible with Swift Package Manager (https://github.com/slsrepo/multimarkdown-spm).
Since then, I used the fork extensively in my apps and other Swift projects (including Server side projects using Vapor and/or NIO).
In November, following the removal of the old outdated package manifest and the changes to the project structure, I updated my fork accordingly and Fletcher suggested bringing the repo up for discussion here for other Swift users so we can establish some "best practices" and validate what works and doesn't in order to merge the fork back into the main repo.
The changes required to allow building with Swift are few, mainly related to the disabling of the token pool and disabling CURL while building for iOS, but to make this change additive and allow building Swift alongside CMake (and not instead of it), The changed source files in the fork live in the "swift" folder instead of "src".
This should also make it easy for Fletcher to automate the changes, by making CMake update the files in the "swift" folder automatically.
This will also make integration with Xcode easier and solve several of the issues linked below, since you wouldn't need to build the project manually any more.
The fork includes 3 Swift Packages: The regular libMultiMarkdown C library, MultiMarkdownSPM which includes Swift functions for the C library (alongside a class useful for implementing syntax highlighting), and mmd-swift which is an executable demo of the packages at work.
Relevant Links
And here is a list of the related issues solved by using the fork:
#88
#89
#92
#95
#101
#170
#187
#199
Looking forward to hear/read your thoughts :)
The text was updated successfully, but these errors were encountered: