On 13-10-11 03:29 AM, Thomas Martitz wrote:
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

Yep, but it's one single place this could happen.

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)


O(1) is pretty OK to me :)

I actually don't think they use hash table for GObject data lists though, rather it's probably some type of binary tree or such but it's still relatively cheap lookup.



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.


? I mean to just literally use a real GPtrArray of notebooks, instead of just toggling between a fixed two of these containers.

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.


I don't doubt it, but what really does it gain over just putting the literal C code? If you are repeating it so much, refactor your code, otherwise, it's just normal part of C. Mutilating C with the preprocessor quite often does not improve the clarity of usability of code.




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.


Yeah, the big cleanup is more of a pipe/future dream for me I guess, so I'll just contribute and help out normally rather than get too involved because I've tried to tackle this myself before using the big cleanup approach, and decided it wasn't worth the effort.


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.


I just mean that the study of such applications is not new, like some canonical texts such as http://en.wikipedia.org/wiki/Design_Patterns, specifically 2.1.

We also have all of those that came before, like borland, ms, anjuta, vim, emacs, eclipse, gedit, mousepad, netbeans, qtcreator, xcode, and 42.0 billion more examples, many of which we can study their source code or even overall architecture and see some good/bad ways to code stuff.

For something more concrete, maybe take Cocoa's document architecture;
https://developer.apple.com/library/mac/documentation/DataManagement/Conceptual/DocBasedAppProgrammingGuideForOSX/Introduction/Introduction.html

It might not be Geany-style C, but a lot these ideas are known and used elsewhere, probably even inherited from the first link. I just suggest we could copy some of them too, rather than looser ad-hoc coding :)



(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.


No comment :)

Cheers,
Matthew Brush

_______________________________________________
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel

Reply via email to