fletcher / MultiMarkdown-6

Lightweight markup processor to produce HTML, LaTeX, and more.
https://fletcher.github.io/MultiMarkdown-6/
Other
621 stars 90 forks source link

Swift Package Manager Support #200

Open slsrepo opened 3 years ago

slsrepo commented 3 years ago

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 :)

fletcher commented 3 years ago

To clarify:

  1. The token pool doesn't have to be disabled based on whether you use Swift or not. It has to be disabled if you are not completely confident in your ability to understand memory management in a multithreaded or "long-lived" application. The token pools provide better performance for the command-line tool, but can be tricky otherwise if you are using the library in other programs. To avoid bugs related to this, I disable token pools in MultiMarkdown Composer and nvUltra.

  2. Similarly, disabling Curl is unrelated to Swift, and only related to whether the Curl library is available on your target platform.

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!!

fletcher commented 3 years ago

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.

slsrepo commented 3 years ago

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:

  1. That each target will have it's own folder (doesn't matter where it is or how it's called, but the default is "\Sources\\")
  2. That all files in the folder be built as is, no template files or other build time processing required.
  3. On Swift projects using the Swift 5.1 and above package manifest, That the package name and repo/folder name match (and It's best practice not to mix - and capital letters in the package name).
  4. If the target is a C/C++/Objective-C target, that public headers be included in a folder called "include" in the target's folder.
  5. If you are mixing Swift with C/C++/Objective-C in an executable target, make sure there's only one main function (or "main.swift" file).

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 #undef (changing the file) and it's recommended for most users to disable the token pool, I included the change in the repo.

Since the repo used to require the flag -Xcc -fbracket-depth=264, I also commented HASH256 for the localized titles, to make it possible to use the package with Xcode.

The last change is just including files like version.h which are built out of the template files with CMake.

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 make swift or something similar before you push changes to the source files.

Hope others can share their thoughts too, maybe Christian (@DivineDominion) can contribute some thoughts :)

DivineDominion commented 3 years ago

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.)

nickasd commented 3 years ago

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

Schermata 2021-02-18 alle 13 35 34

Is there something I can provide to help you understand what's going on?

slsrepo commented 3 years ago

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.)

Thanks Christian! :) Creating a target that supports the token pool sounds like a good solution, but we need to find a naming scheme for that - libMultiMarkdown and libMultiMarkdownTokenPool? Should libMultiMarkdown be the one with the pool enabled? It's currently the one where the pool is disabled (I think it should stay like this, since the majority of people should probably keep the pool disabled).

The changes are indeed minimal, and the library behaves exactly the same as it would when built with CMake. You can see and compare the exact changes at the repo.

If you have a bit of time, you can try integrating it in a project and see how it works for you :)

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

Schermata 2021-02-18 alle 13 35 34

Is there something I can provide to help you understand what's going on?

Hi Nick, thanks for trying the package :)

If you right click (or control click) on the error and click "Reveal in Log" - what is the exact error that shows on the log?

Edit: Also, what is the version you integrated? 6.6.0? Thanks :)

nickasd commented 3 years ago

There's not much more:

~/myProject/Markdown.swift:12:8: error: no such module 'MultiMarkdownSPM'
import MultiMarkdownSPM
       ^

And yes, I'm using version 6.6.0.

slsrepo commented 3 years ago

Thanks for that, can you confirm that the libraries are included in your target, in the project settings, like this?

Screen Shot 2021-02-18 at 23 12 56
nickasd commented 3 years ago

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: 1) When running my app with the Swift module for the first time, it crashed with this error:

malloc: Heap corruption detected, free list is damaged at 0x600000763c40
*** Incorrect guard value: 62814420993
malloc: Heap corruption detected, free list is damaged at 0x60000076d7e0
*** Incorrect guard value: 36507746304
malloc: *** set a breakpoint in malloc_error_break to debug
malloc: *** set a breakpoint in malloc_error_break to debug
malloc: Heap corruption detected, free list is damaged at 0x60000076d7e0
*** Incorrect guard value: 36507746304

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).

2) When calling MultiMarkdown.parse(substring(with: range))!, many things are logged to the console:

dnssd_clientstub ConnectToServer: connect() failed path:/var/run/mDNSResponder Socket:21 Err:-1 Errno:1 Operation not permitted
[connection] nw_resolver_create_dns_service_locked [C5] DNSServiceCreateDelegateConnection failed: ServiceNotRunning(-65563)
Connection 5: received failure notification
Connection 5: failed to connect 10:-72000, reason -1
Connection 5: encountered error(10:-72000)
Task <7AA92B76-FF70-46CD-AA26-F1FD286E4E9A>.<0> HTTP load failed, 0/0 bytes (error code: -1003 [10:-72000])
NSURLConnection finished with error - code -1003
dnssd_clientstub ConnectToServer: connect() failed path:/var/run/mDNSResponder Socket:21 Err:-1 Errno:1 Operation not permitted
[connection] nw_resolver_create_dns_service_locked [C6] DNSServiceCreateDelegateConnection failed: ServiceNotRunning(-65563)
Connection 6: received failure notification
Connection 6: failed to connect 10:-72000, reason -1
Connection 6: encountered error(10:-72000)
Task <0B81A83A-A047-4C94-9D9E-B463DED6A6BB>.<0> HTTP load failed, 0/0 bytes (error code: -1003 [10:-72000])
NSURLConnection finished with error - code -1003
slsrepo commented 3 years ago

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).

nickasd commented 3 years ago

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 (MultiMarkdown.parse(substring(with: range))!) and replacing it with a hard-coded string, those lines don't appear anymore.

slsrepo commented 3 years ago

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.

nickasd commented 3 years ago

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.

slsrepo commented 3 years ago

Another thing I just noticed, Have a look at your substring(with:) function. You can also try using MultiMarkdown.parse("**test**"). You can also try seeing if you get a different result using the C Function instead of the Swift one (included in libMultiMarkdown instead of MultiMarkdownSPM) - this is the code behind MultiMarkdown.parse:

/// 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
    }
nickasd commented 3 years ago

(I also get the same console output when using the Objective-C calls.)

slsrepo commented 3 years ago

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.

I can definitely add that to the README :)

Glad you got it resolved :)

(Funny how we posted in the same second :P)

slsrepo commented 3 years ago

(I also get the same console output when using the Objective-C calls.)

Cool :) Just to clarify, that comment above (detailing how MultiMarkdown.parse works) was sent before your comment came in :)

slsrepo commented 3 years ago

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? :)

DivineDominion commented 3 years ago

@slsrepo The current version disables the token pool by default

slsrepo commented 3 years ago

@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 :)

slsrepo commented 3 years ago

@DivineDominion Done. You can give it a try now :)

nickasd commented 2 years ago

@slsrepo I opened an issue on the official repository a month ago, but it wasn't answered. Are you still maintaining it?

slsrepo commented 2 years ago

@slsrepo I opened an issue on the official repository a month ago, but it wasn't answered. Are you still maintaining it?

@nickasd Hi Nick, Yes, I'm still maintaining the repo :) I missed the issue you opened. Sorry about that! Check the issue for my reply :)