ruddfawcett / Notepad

[iOS] A fully themeable markdown editor with live syntax highlighting.
http://rudd.fyi/notepad
MIT License
878 stars 106 forks source link

Style boldItalics not rendered correctly #51

Open funkenstrahlen opened 5 years ago

funkenstrahlen commented 5 years ago

The following example should render as bold and italics at the same time:

***foobar***

Like this: foobar

However it is only rendered in bold style.

funkenstrahlen commented 5 years ago

This is actually supported, but no used in any of the example theme files. Therefore I close this.

funkenstrahlen commented 5 years ago

After some more testing I have discovered a bug that arises from a library design flaw.

In the current implementation ***foobar*** matches three regular expressions for bold, italics and boldItalics. It only renders in the correct style if the regex for boldItalics is processed after the other ones. But this is currently not guaranteed in the code. Therefore it works sometimes and sometimes does not.

The styles are applied here:

        for (style) in theme.styles {
            style.regex.enumerateMatches(in: backingString, options: .withoutAnchoringBounds, range: range, using: { (match, flags, stop) in
                guard let match = match else { return }
                backingStore.addAttributes(style.attributes, range: match.range(at: 0))
            })
        }

The order of styles in theme is dependent on the JSONSerialization when reading from disk. As styles in the theme JSON is defined as a dictionary no order is guaranteed.

I think to fix this issue the boldItalics case needs to be rethought completely because it requires a different render architecture. Currently I do not have an idea on how to solve this.

funkenstrahlen commented 5 years ago

Maybe the issue can be fixed by improving the regular expression for bold, italic and boldItalic.

ruddfawcett commented 5 years ago

Thanks for bringing this to my attention; the styles do need to be reworked a bit and are probably a part of a massive overhaul that includes separating tokens such as *, **, ***, etc. (à la #28). I will think on this and get back to you.

funkenstrahlen commented 5 years ago

Awesome! Let me know if you have an idea how to implement this. I am open to help writing the required code.

DivineDominion commented 5 years ago

I found it works well if you don't just command it to e.g. "set this to italics", but instead "fetch the current font and add the italic font trait".

For bold and italics, I call embolden and italicize:

    public func regularize(range: NSRange) {

        let font = self.font(at: range.location + range.length / 2)
        self.addStyleAttribute(.font, value: font.regularized(), range: range)
    }

    public func embolden(range: NSRange) {

        let font = self.font(at: range.location + range.length / 2)
        self.addStyleAttribute(.font, value: font.emboldened(), range: range)
    }

    public func italicize(range: NSRange) {

        let font = self.font(at: range.location + range.length / 2)
        self.addStyleAttribute(.font, value: font.italicized(), range: range)
    }

    public func font(at location: Int) -> UniversalFont {

        return self.styleAttribute(.font, at: location) as? UniversalFont
            ?? UniversalFont.systemFontOfDefaultSize
    }

With this:

public typealias UniversalColor = NSColor
public typealias UniversalFont = NSFont
public typealias UniversalFontDescriptor = NSFontDescriptor

extension UniversalFont {

    func italicized() -> UniversalFont {
        return NSFontManager().convert(self, toHaveTrait: .italicFontMask)
    }

    func emboldened() -> UniversalFont {
        return NSFontManager().convert(self, toHaveTrait: .boldFontMask)
    }

    func regularized() -> UniversalFont {
        return NSFontManager().convert(self, toHaveTrait: [.unboldFontMask, .unitalicFontMask])
    }

    static var systemFontOfDefaultSize: UniversalFont {
        return UniversalFont.systemFont(ofSize: UniversalFont.systemFontSize)
    }
}
ruddfawcett commented 5 years ago

@DivineDominion love that approach! @funkenstrahlen if you want to take a look...

funkenstrahlen commented 5 years ago

I just took a look at this and still do not know how to integrate this in to the existing structure.

Currently it works like this:

  1. The Theme file is parsed. This creates an array of Style objects
  2. Each Style object represents a regex and all attributes for the NSTextStorage to apply to the matching text range

Problems with this approach:

Merging of font traits settings for two Style objects (matching the same regex) does not work, because font traits are part of the font attribute and are therefore not merged together but replaced when calling backingStore.addAttributes(style.attributes, range: match.range(at: 0))

This could be solved by applying font traits separately after all other styles have been set. However this is not possible because currently there is no access to the traits attribute in the json theme file other than through Styles.

funkenstrahlen commented 5 years ago

Maybe there is a way to build this just for bold, italic and boldItalic elements. I could create exceptions for these elements and add the font traits manually. However this means loosing the option to customise font traits in the theme definition file for headlines for example.