Linked Image Mechanism: Cleanup!

[jleandroperez]

Details:

This PR introduces no new functionality. We're:

1. Encapsulating the linkedImage parsing mechanism, into it's own method
2. Moving the textAttachment constant over to a String extension, for convenience
3. Adding a small method to ElementNode (and few unit tests!)

To test:

1. Verify that the unit tests are green
2. Add a linked image <a href="www.wordpress.com"><img src="www.something.com/img"></a>
3. Try toggling back and forth, and verity that the HTML is preserved

Needs Review: @diegoreymendez
Thanks in advance!

[SergioEstevao]

Loved this little touch!

[jleandroperez]

I absolutely LOVE 'extensible enums' (that's this trick's name!)

[SergioEstevao]

Great to have this tests on.

[jleandroperez]

Thank you sir!

[SergioEstevao]

Great work here!

[diegoreymendez]

As "children" is the plural form, we should change to "child" here and in the lines below.

[diegoreymendez]

/s/children/child

[diegoreymendez]

/s/onlyChildr/onlyChild

Want to avoid flooding the PR but search for other occurrences below.

[diegoreymendez]

Closing { goes in the same line as the declaration preceding it.

From https://github.com/wordpress-mobile/swift-style-guide#braces:

Opening braces should be preceded by a single space and on the same line as the declaration. The exception to this rule is when enclosed within parentheses: no preceding space is required.

[jleandroperez]

Our guidelines don't fully apply to this statement, because the guard clause has multiple lines / conditions.

Moving the else to the preceding line, would have the sad effect of forcing us to further increase the indentation level of the snippet that goes inside the else.

IMO this way we truly improve readability, and avoid increasing indentation.
I'll submit a formal request to add this to our guideline!

[diegoreymendez]

I still think this looks very inconsistent with how we format the rest of our code, as we don't separate else statements from preceding declarations anywhere else.

The fact that auto-indent adds an extra tab should not affect how we format the code.

That said, as this won't affect the logic in the code, we can merge as it is.

[diegoreymendez]

In order to aim for single-responsibility would you mind splitting this method into two steps (retrieval, filtering)?

This'd help ensure our methods are easier to compose, and also easier to read and understand.

Example:

/// This one returns first child found, if it's the only one.
///
func onlyChild() -> ElementNode? {
    guard children.count == 1 else {
        return nil
    }

    return children.first as? ElementNode
}

/// This one returns the first child found, if it's the only one and it matches the specified type.
///
func onlyChild(ofType type: StandardElementType) -> ElementNode? {
    guard let child = onlyChild(),
        child.isNodeType(type) else {
            return nil
    }

    return child
}

[diegoreymendez]

Just one more missing "children" replacement, but otherwise approved.

Nice work.

[diegoreymendez]

/s/children/child

[jleandroperez]

Thank you both!