Consistency of EditContext/SuperEditorContext as well as Event/Change #2379
Replies: 1 comment
-
Re: The two different contexts. I think Re: Events vs changes - I'll need to revisit all of those. But it's very possible that we have inconsistent naming for no reason. Edit events can apply to any kind of change. Document changes, as the name implies, refer to specific changes that occurred within a document. If you'd like to compile a list of recommended changes, that might be helpful. However, I wouldn't suggest putting up PRs with refactors because we'll need to consciously pick appropriate times to further break things, rather than do it arbitrarily over a period of time. |
Beta Was this translation helpful? Give feedback.
-
I'm using super_editor to develop an outline editor package. It turns out that doing this with components instead of implementing a custom layout, as @matthew-carroll suggested, works nicely so far and I'm getting along fine. I stumbled on a few things that I find confusing and would like to mention them here:
There is a class
EditContext
; when it is used for a parameter, that is generally callededitorContext
, noteditContext
. And there is a classSuperEditorContext
, where parameters are often callededitContext
, noteditorContext
orsuperEditorContext
. This is, for me, extremely confusing. I would suggest naming parameters of type EditContext "editContext" and those of type SuperEditorContext "superEditorContext". In general, I feel that an explanation of the relationship of those two classes would help developers. As a side note: AsSuperEditorContext
is obviously specific to theSuperEditor
widget, I sort of expected this to be passed to theattach
method of plugins, so they could for example query the layout; however, I managed to get along without.A similar inconsistency in naming I find with the concepts of
Event
andChange
in the edit pipeline. There is a class hierarchy DocumentChange > NodeDocumentChange > NodeInsertedEvent. However, there is also an EditEvent > DocumentEdit hierarchy. If I understand it correctly, there are events, each of which may incorporate changes. Then I think, "NodeInsertedEvent" (which inherits NodeDocumentChange) for example should be "NodeInsertedChange"; "DocumentEdit" should be "DocumentEditEvent". What makes it more confusing is that in the Reactions that I find in the package, there is typically aList<EditEvent> changeList
and then a "for (final event in changeList)". This should be an "editEventList".I understand this may be established names for quite a while, but with a lot of breaking changes in the next version, I feel things like this should be made consistent now. If you feel combing through the code and refactoring these things is at the moment rather tedious for the core team (which I agree is working on more important stuff), I can happily go for this and send you pull requests, or first open issues. I was unsure if this should have been an issue rather than a discussion.
Beta Was this translation helpful? Give feedback.
All reactions