Am 11.10.2013 12:07, schrieb 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.
That pointer needs to be updated properly when the doc is moved. I rather save that and walk the tree, it's cheap enough. (g_object_get_data() is a hash-table lookup which isn't exactly free either)
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.
There is no "notebooks_arr" in the current code. Instead there is a macro which makes it easy to transition to such an array later on.
Seriously, if foreach_* macros are so frowned upon, why is Geany still full of them? I'm literally just following existing Geany-style with that macro.
I personally like these macros, they are easy to understand and nicely short. I've also seen them used in other projects.
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).
I would still love to collaborate with you and other devs, even if I'm not going to make the uncertain (for me, anyway) big cleanup personally.
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).
Loose statement, see below.
- 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.
That's a very loose statement. Just saying "like the other IDEs" doesn't give me a vision. I haven't looked at their code so I can't say in what way their code is better than Geany's. You have probably more experience you worked on Mousepad (although not an IDE) and perhaps others? And I'm not going to see what others do just for the sake of change.
(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...
Most code bases are like this. I've seen worse.
Best regards.