[Geany-Devel] splitwindow2

[Matthew Brush]

On 13-10-11 02:23 AM, Thomas Martitz wrote:
> Am 11.10.2013 07:01, schrieb Matthew Brush:
>>
>>> notebooks". 99% of the now hardcoded places will work fine, e.g. I'm
>>> using gtk_widget_get_parent(sci) to get a doc's notebook and implement a
>>> foreach_notebook() macro. So this should make it really trivial to
>>> support even more than 2 notebooks.
>>>
>>
>> gtk_widget_get_parent(sci) makes a big assumption; that the Scintilla
>> will be packed directly into a notebook. Plugins can easily break this
>> assumption by reparenting the Scintilla widget, and even my
>> document-messages branch moved each Scintilla into a VBox inside each
>> tab so it could pack a GtkInfo bar above it. I think we could get rid
>> of this assumption by designing the code better.
>
> Okay, I have written a function that walks the tree to find the parent
> notebook. So this assumption is no more. Thanks for the hint.
>

You don't need to walk any tree's, if you need to notebook from the 
scintilla, just store a pointer to the notebook in the scintilla, ex. 
using g_object_set_data() or more painfully by subclassing it.

>>
>> Please don't write another foreach_ macro. Lets make it so we can use
>> normal C code. If you need a macro like foreach_notebook() it's
>> probably because the code it's hiding is crappy underlying code, IMO.
>
> I like foreach_*, and many other people do. I guess it's subjective.
> Plus it will make the transition from "main and secondary notebook" to
> "array of N notebooks" easier.
>

What's the problem with just using C code?

for (size_t i=0; i < notebooks_arr->len; i++) {
   // just normally use elements in the GPtrArray (as example)
}

Such macros not only make for weird APIs but also do weird stuff with 
arguments since it's just textual replacements.

>
>>
>>> But still, I personally don't have a need for more than 2 notebooks.
>>> Plus I see it will be difficult to support, because you need to nest
>>> GtkPaneds. Since you suggest arbitrary splits you can do this only
>>> programmatically which sounds like a major headache to me (especially
>>> w.r.t. saving/restoring state). For now I want to concentrate on lifting
>>> the hardcoded single notebook without ending up in the same situation
>>> for two ones and without worrying about the heavy case of arbitrary
>>> notebook placing. Then we can continue from there on with more
>>> sophisticated stuff.
>>>
>>> However if you have already successfully implemented this in a toy
>>> editor I welcome you to join the fun.
>>>
>>
>> Just to be clear, regarding the split window feature in my toy editor,
>> I only coded the UI logic, not any of the storing config of what panes
>> were open or what documents they had in them and more complicated
>> stuff like this. But the UI was indeed quite neat. SublimeText also
>> has a similarly flexible split pane system.
>
> Okay, so you stopped ater the easy parts :P
>

Yep :)

>>
>>
>> All of those functions in document_ module shouldn't care the least
>> about GTK+ or even notebooks, IMO. This is the type of stuff I want to
>> cleanup.
>>
>>> There are also many direct uses of main_widgets.notebook, changing these
>>> might (or might not) affect plugins.
>>>
>>
>> IMO, if you're going to break the plugin API anyway, I say make it
>> good and we'll fixup core and GP plugins and make a message on the
>> mailing list for other users to know what broke and how to fix it in
>> their own personal plugins.
>
>
> It's not yet clear to me how bady (or if at all) the API is going to
> break..
>
>
>>
>>>
>>>>
>>>>> I already have an experimental version up and running that doesn't
>>>>> even
>>>>> require all that much changes[1] and it seems to work nicely. See
>>>>> this[2] screenshot.
>>>>>
>>
>> IMO, least amount of changes is not necessarily a virtue, and can even
>> lead to further gluing of the code together making future changes even
>> harder.
>
>
> But it's better than not getting the job done at all because people want
> to much at once.
>

Yeah, but I wonder. Sometimes just patching on more changes adds to the 
number of bugs and code weirdness at least as much as just refactoring 
the code being patched on to. Of course if you just want to tack on a 
quick feature this is desirable, but if you want to improve the overall 
code quality, it's arguable.

>
>>
>>>>> So, any opinions?
>>>>>
>>>>
>>>> It sounds like you already are planning some of this, but it would be
>>>> nice to cleanup a lot of the assumptions/hardcoded stuff/weirdness
>>>> while making these fairly intrusive changes. For example:
>>>>
>>>> - The relationship between documents and notebooks they're in, as you
>>>> discussed. As Lex mentioned, it would be nice to not make too much new
>>>> hard-coded assumptions about other documents only being allowed in
>>>> another notebook, but rather making it extensible to support multiple
>>>> windows in the future. Also as I mentioned, it would be nice to not
>>>> make any hard-coded assumptions about only having two split
>>>> notebooks[1].
>>>
>>> gtk_widget_get_parent(doc->editor->sci) without hardcoding. I'm using
>>> this where possible.
>>>
>>
>> You're still hardcoding, just a different assumption as mentioned above.
>>
>>>>
>>>> The notebook of a doc can be retrieved via - The relationship
>>>> between documents (models) and Scintilla (views),
>>>> they should be almost completely independent. There shouldn't need to
>>>> have document->editor->scintilla, the document needn't care what view
>>>> it's in, only the reverse. I have no idea where GeanyEditor fits into
>>>> this, I've never understood why it exists; it's not a view, it's not a
>>>> model, and it's not really a controller either, it's like part wrapper
>>>> over Scintilla, part extension of GeanyDocument or something like this
>>>> I guess?
>>>
>>> Sorry, I think this item is out of scope for this work. FWIW, I don't
>>> quite understand the deal about GeanyEditor either.
>>>
>>
>> It's not out of scope, but it is way more work than I think you're
>> personally prepared to do. It's basically the reason no one fixed the
>> split window before or worked to moving it into core, because it was
>> too much work with the current way it's coded. If this kind of stuff
>> gets cleaned up first, the split window thing becomes trivial (as well
>> as multi-window, etc). I would've fixed up split window myself 10x by
>> now if I didn't morally have to cleanup all this code to do it correctly.
>
>
> See, if you make this kind of cleanup a hard requirement that no
> progress is going to happen, for the reasons you mention. Can't we just
> progress with managable portions of cleanups/workload, considering our
> already heavily limited developer manpower?
>

Yep, that's why my previous message, I'll try not to interfere here too 
much since no one is interested in the type of stuff I'm talking about 
probably (cleanup up the code en masse).

> The cleanup you propose would be impossible for me to do anyway, because
> I have no vision how the end result would look like. Without such a
> vision I wouldn't know where to start and where to end. As I said, I
> don't know about the GeanyEditor deal but I don't have a problem with it
> either. And splitwindow can be done with the current structures. So I
> maintain that your proposal is out of scope for my work. The same goes
> for your other cleanup proposals.

The patterns and paradigms used in IDE software and editors are not new, 
we aren't paving new ground here, usually we're just fighting it by not 
using the tools at our disposal for artificial reasons (totally off-topic).

>>
>>>> - Encapsulating the GeanyDocument so that plugins can't mess with
>>>> read-only members. For example, it makes no sense to allow plugins to
>>>> change doc->read_only, or doc->file_name (one of them actually does
>>>> this). It would be nice to make the API consistent here, like we have
>>>> document_set_text_changed() to mark the document as ditry/clean, but
>>>> there's no getter like document_get_text_changed(), which is
>>>> inconsistent and it allows plugins to seriously break Geany if they
>>>> aren't careful. This one is of course fairly off-topic and could be
>>>> attacked separately afterwards, I just thought it was worth mentioning
>>>> since you talked about needing to break the plugin API, it might be
>>>> useful to improve it during the breakage.
>>>>
>>>
>>> Also unrelated. I agree that big API breakage should be as rare and
>>> concentrated as possible (i.e. within a single release) but not as part
>>> of a single change. Let's not blow this splitwindow work up with
>>> unrelated changes. This would just make review impossible and cost more
>>> time to get done.
>>>
>>
>> The fact that it "blows up" into more work is exactly why we should
>> bite bullet instead of keep "just making it work with minimal
>> changes", when the code is factored well, making changes becomes
>> easier. But I digress somewhat (as usual).
>>
>> I withdraw my request to help out with this because I don't think
>> you're interested in doing some of the bigger underlying changes I was
>> interested in doing before attempting to fixup split window, and I
>> don't want to drag you down. As long as you keep in mind the stuff
>> mentioned, and don't make any additional same type of assumptions that
>> were made and evolved previously, I think it won't be such a big deal
>> to do the stuff I was talking about underneath your changes as a
>> separate initiative later on.
>
>
> Right, I'm not interested in doing the bigger cleanups. For three
> reasons: (a) my time is limited and I want to get splitwindow done more
> than anything else (w.r.t. to Geany) currently. (b) unlike you I have no
> vision how the cleanup should look like. (c) I have no problem with the
> current code base (yet).
>

(a) yep, just throw another patch on it, it's not going to much change 
situation and it at least adds a highly desirable feature.

(b) I usually just study how other IDEs and editors work and are coded. 
As mentioned, this is not new territory, we have footprints to follow.

(c) A lot of it is intertangled and is the result of starting with no 
design and then tacking on another feature, and another feature and so 
on. Don't get me wrong, it works very well, but anytime you go to make 
any non-trivial change to the source you end up opening a whole can of 
worms or just patching on another hack because you don't have a week to 
refactor the code, I'm as guilty as anyone...

> You aren't willing to do the cleanups either, as you noted before. So
> I'm not actually sure what you're after.
>

I'm willing to actually *do* the cleanups, I'm just not up to the task 
of bikeshedding over every line of code in excruciating detail by people 
who won't even add your remote and test and push you some changes for 
stuff they don't like (I don't exclude myself here). It's a grueling way 
to accept contributions, which is why I said I will mostly stay out of 
the way.

>
>>
>>>
>>>> I actually use the current crippled split view *extensively* so I
>>>> would also be really interested in helping out with this. If it was
>>>> useful we could make a branch on the main repository to work from and
>>>> get more visibility with it probably.
>>>>
>>>
>>> Fine with me. You can also make PRs on my github until this is set up
>>> (but beware that I still amend and rebase) :)
>>>
>>
>> I can still make a branch for this but as mentioned above I won't be
>> contributing much to it as it stands. I will be a good tester though
>> because I *really* want a fully functional split window :)
>
>
> I appreciate testing. But right now it's still a long road.
>
>>
>>>
>>>> Cheers,
>>>> Matthew Brush
>>>
>>> Thanks for your input.
>>>
>>
>> Sorry to be such a downer, I just really hate seeing this vicious
>> circle of adding more and more code ontop of other code that needs to
>> be refactored, making everything all intertwined and hard to hack on.
>
>
> Well, it needn't get worse that status quo. See it as part of
> incremental cleanup. Why are you so pessimistic about that?
>

It truly isn't worse than the status quo, I just debate that the status 
quo needs to continue forever.

Cheers,
Matthew Brush