PreviousTracker indexSee it online !

(27/27) 626 - Refactoring with a new BufferManager, ViewManager

Hey,
I could commit the patch directly but as it is big I prefer suggesting it to you for reading if anyone has the courage :)
The idea is to remove some code of the big jEdit class and handle the chained Buffer and View list in separate classes.
With some changes we can use from modern JVM such as streams.
I know it might be slower than a chained list, but I think it's a problem of nanoseconds and it can make the code much easier to read.
What do you think of the idea ?

Submitted kpouer - 2020-04-06 16:52:52.945000 Assigned
Priority 5 Labels
Status open Group None
Resolution None

Comments

2020-04-06 18:18:02.028000
daleanson

I like it. The patch applied cleanly. There are 4 deprecation warnings that weren't there before, all 4 are calls to jEdit._getBuffer. Should those be calls to bufferManager.getBuffer? I always worry about changes like this causing problems with plugins, I'll let you know if I see any issues.

2020-04-06 19:07:34.469000
kpouer

In fact I was thinking a lot about deprecating all methods like getBuffer(), getActiveView() & others and ask to use the new managers, it would help reducing the size of jEdit class, but the problem is plugins, and the fact it is convenient to have all those methods centralized in jEdit.
So I was not sure what was the best choice.
Anyway as long as we don't remove deprecated methods it causes no problem.
However removing methods that are deprecated since several version of jEdit would be nice to get a cleaner codebase don't you think ? (not related with that patch of course).

2020-04-07 00:57:00.430000
daleanson

I agree about removing the deprecated methods at some point, the downside
is quite a few of the useful plugins are not really being maintained
anymore.

On Mon, Apr 6, 2020 at 7:07 PM Matthieu Casanova <
kpouer@users.sourceforge.net> wrote:

> In fact I was thinking a lot about deprecating all methods like
> getBuffer(), getActiveView() & others and ask to use the new managers, it
> would help reducing the size of jEdit class, but the problem is plugins,
> and the fact it is convenient to have all those methods centralized in
> jEdit.
> So I was not sure what was the best choice.
> Anyway as long as we don't remove deprecated methods it causes no problem.
> However removing methods that are deprecated since several version of
> jEdit would be nice to get a cleaner codebase don't you think ? (not
> related with that patch of course).
> ------------------------------
>
> * [patches:#626] <https://sourceforge.net/p/jedit/patches/626/>
> Refactoring with a new BufferManager, ViewManager*
>
> *Status:* open
> *Group:*
> *Created:* Mon Apr 06, 2020 04:52 PM UTC by Matthieu Casanova
> *Last Updated:* Mon Apr 06, 2020 06:18 PM UTC
> *Owner:* nobody
> *Attachments:*
>
> - bufferManager.patch
> <https://sourceforge.net/p/jedit/patches/626/attachment/bufferManager.patch>
> (78.7 kB; application/octet-stream)
>
> Hey,
> I could commit the patch directly but as it is big I prefer suggesting it
> to you for reading if anyone has the courage :)
> The idea is to remove some code of the big jEdit class and handle the
> chained Buffer and View list in separate classes.
> With some changes we can use from modern JVM such as streams.
> I know it might be slower than a chained list, but I think it's a problem
> of nanoseconds and it can make the code much easier to read.
> What do you think of the idea ?
> ------------------------------
>
> Sent from sourceforge.net because you indicated interest in
> https://sourceforge.net/p/jedit/patches/626/
>
> To unsubscribe from further messages, please visit
> https://sourceforge.net/auth/subscriptions/
>

alternate (2.7Kio)

2020-04-07 15:27:36.663000
kpouer

If we have the sources of those plugins and the developper is not available I would be happy to take a look.

2020-04-08 14:33:17.448000
kpouer

In fact about the two deprecated calls to _getBuffer(), it would be easy to expose the method in the BufferManager interface and call it, but I was not sure we want to have 2 methods getBuffer and _getBuffer.
It is low level and maybe only one method should be exposed (but I was not sure how to do it right)

2020-04-11 07:55:10.542000
kpouer

Hello,
I fixed the two deprecated api calls. I did it without exposing _getBuffer() publicly because I think it might be disturbing in the api to have two methods
getBuffer() and _getBuffer(), but if many plugins needs both, then I can expose it too.
What do you think ? Should I apply the patch ?

bufferManager_2.patch (81.4Kio)

2020-04-14 20:01:24.895000
kpouer

I finally committed the patch.
it was possible to add a convenient forEach() api directly in the BufferManager, but I prefered not and return a List of buffers because iterating over buffers needs synchronization and we don't want a slow forEach callback break everything.