extension NodeInitializable should be public?

[UberJason]

I'm trying to make a data type conform to NodeConvertible. NodeConvertible is made up of NodeInitializable and NodeRepresentable, where the method makeNode() is defined in NodeRepresentable, and the method init(node: Node, in context: Context) is defined in NodeInitializable.

NodeInitializable has a protocol extension which defines a default initializer, so that I should only need to implement makeNode(), if I'm understanding correctly. However, when I do this, my data type still doesn't conform to NodeConvertible.

I believe the problem is that the NodeInitializable extension should be public, but it isn't. Thus, my own data type in my project which imports Node can't see that there's actually a default implementation of the initializer already there. Is my analysis correct, and would you be willing to make that change?

EDIT: The other possibility is that the NodeInitializable extension actually only defines init(node: Node), and not init(node: Node, in context: Context), so my data type still needs to implement the latter.

[loganwright]

@UberJason the edit is correct, the extension isn't a default, but rather a convenience method since context is only used in practice for a few different use cases. A lot of the time Foo(node: is all enough.

[UberJason]

So my question I guess would be, since context is only really used for a few different use cases, would it make sense to expose the NodeInitializable extension with its default initializer as public, so other data types conforming to NodeConvertible can take advantage of it?

[loganwright]

@UberJason I'm not sure I follow, the default initializer is public, is it not? Maybe I'm missing something if you have more info.

[UberJason]

Sorry, I misspoke. I meant, would it make sense to expose the NodeInitializable extension and its convenience method as public. Since, as you said, that convenience method is often enough. It's a minor thing, just wondering.

[loganwright]

@UberJason all of the methods are exposed, I think what you're asking for is to put them both on the protocol so that you can choose which one to override, like:

protocol Bar {
    init(node: Node, in context: Context) throws
    init(node: Node) throws
}

Unfortunately, there's no good way to say "Use one of these functions, but not both", so it's difficult to know which is implemented, and it'd require both. To get around this, everyone is required to implement the full initializer w/ Context, and the convenience can be used after that point. Sorry it can't just be the clean and simple way only for now, but this little addition helps us broaden our support for some of the more complex situations.

[UberJason]

No worries, it was minor. :) Also I was working from 0.17 at the time, it may have changed in 0.18 (haven't had a chance to go back and check).

[loganwright]

@UberJason it hasn't unfortunately, but if something changes that we're able to achieve this, we'll definitely revisit!