Create base TreeMessage for tree messages?

[rodrigogiraoserrao]

OptionList has a OptionMessage that is inherited by the two OptionList messages.
Is it worth doing the same for the tree messages? There are 4 messages whose code is exactly the same, the only thing that changes is the name of the class.

[davep]

IIRC I made that change once but it got reversed. Might be worth looking back through history.

I think the objection was to do with impact on documentation, so perhaps we're in a better place now.

(To be clear, I'm massively pro common parent class)

[rodrigogiraoserrao]

IIRC I made that change once but it got reversed. Might be worth looking back through history.

You are right, and I was the one who removed the base class 😆 f34685b

I think the objection was to do with impact on documentation, so perhaps we're in a better place now.

I see the point. It isn't super obvious what attributes the OptionList messages have.
Maybe if we add a sentence in the docstrings for those messages to check the base class, then we can inherit from those messages and we can, once more, have a TreeMessage base class.

Thoughts, @willmcgugan?

[rodrigogiraoserrao]

(If we decide against a TreeMessage, we should probably split OptionMessage too.)

[willmcgugan]

I think with @davep 's change to the @on decorator it makes sense to have common base classes.

The docs issue is annoying.

[rodrigogiraoserrao]

The docs issue is annoying.

Not being super clear what attributes the messages have?

[willmcgugan]

Exactly. The indirection means that some people will miss the attributes in the base class.

[willmcgugan]

It probably does make some sense to have a common base class. But it doesn't seem all that useful: I can't see a reason why you would want to respond to multiple tree events in a single handler.

Closing this, because it just doesn't seem worth changing, unless we have an identiable need for it.

[github-actions]

Don't forget to star the repository!

Follow @textualizeio for Textual updates.