On 13-10-10 06:51 PM, Lex Trotman wrote:
[...]

  - state is not saved/restored across Geany restarts
- it's completely awkward because the other view shows a doc that's also
in the main view, editing in the other view will change the main view at
the same time.


I think this is intended and I find it useful personally. I don't think it
should be taken out, IMO.


If this does not complicate too much, IIRC you found that Scintilla wasn't
*that* automatic when the same document was displayed in two views.  Also
it impacts on areas like saving status, each open file can be displayed in
more than one location, have more than one scroll and cursor position etc.
  Based on your experience you and Thomas should look at how much
complication it adds.  If its too much maybe make it phase two.


The reason no one has worked on the split window much before is exactly because the core of code evolved like you say "if this does not complicate too much" so ends up being non-modular or re-usable and has lots of built in assumptions and stuff gluing the various pieces together. Understandably, no one really wants to spend two days to refactor some code while adding a small feature that would otherwise take a few hours, but it ends up making it very hard to do stuff like split window or separate windows or future stuff we haven't thought of yet. It's like a chicken and egg situation.

[...]


  New docs will be opened in the primary one by default, but can be moved
to the secondary one at will. This means that no document can be shown
in both at the same time which makes it a loss less awkward. It also


As mentioned above, this is actually a feature and IMO it should be kept
unless there is some serious technical reason that makes it impossible.
Scintilla can easily be showing the same document in two views and it keeps
them in sync (see SCI_SETDOCPOINTER and friends).


As above.


As above.


  - anything else?


Even though I usually rant against hard-coding language specific stuff
into the core, I think it would be extremely useful to have an option to
open header/implementation file in the other view. For example if I'm
editing foo.c, it could look for foo.h (either being open or in same
directory), and open it in the other view. It could work the other way
around as well. I think it'd probably only be really useful for C, C++ and
Obj-C filetypes, although I don't know that many languages, so aybe some
others could benefit as well.


Yes, its a good idea, but in core it should be configurable to some sort of
"Open related file(s)".  But its not part of the split-window changes and
should be a separate change set.


I think Thomas is right, this could be done by plugins after the changes being discussed.

  [...]


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:


Cleanups should be separate from feature additions, they should not be
mixed together in the one change-set.  If you need to do things like you
list below to support a feature addition they should be done first as a
separate git branch that can be merged without split-window.


That is *precisely* what I'm driving at, without having said it explicitly. Let's not just patch together a working solution with the bare minimum of work ontop of what exists currently (not that this is at all what is being suggested anywhere), but lets clean it up a lot first, to make stuff like split window trivial to write, just GTK+ widget code at that point. Even GtkApplication makes the multi-window thing trivial if the code is factored correctly. At some point in the future we could use that.




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

- 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?


I think editor exists because it grew to exist, rather than there was some
concious design decision based on MVC separation.  Thats why it spans that
particular breakup.


I guess it is kind of a controller, but it has some other stuff mixed in which always makes me confused. It might be useful to sort this out as well, since GeanyEditor is closely intertwined with GeanyDocument and Scintilla view, which is exactly what should be cleaned up to make stuff like split view or separate windows easier to write.



- The lifetimes of documents. I don't see any reason to recycle a fairly
small structure like GeanyDocument, especially since we basically set it up
and tear it down each time anyway. I doubt the overhead of freeing an old
GeanyDocument structure and allocating a new one later is worth the
contortions it causes in code and the weirdness in the plugin API.


The idea sounds correct, not sure just how much it takes to change it.  If
split-window doesn't need to change it, it isn't *that* big an issue IMHO.
  But anyway its not part of split-window.


As above.


- This follows with above, document_get_current() should *never* return
NULL. It makes absolutely no sense to me to allow having Geany open without
a document open. It'd be like having Firefox open with no tabs/webpages
open. Either it should open a blank untitled document when the last one
closes (this option exists already IIRC) or Geany itself should just close
(probably too annoying :) These last two would get rid of weirdness like
doc->is_valid, DOC_VALID(), "documents" macro wrapping documents_array,
foreach_document() macro to iterate documents, etc.


Nice idea, provided its semantics work for multiple views.  Does each view
get its own "untitled" or what? Anyway should be completely separate change
set, its not actually part of split-windows.  If it makes split-windows
significantly easier, then do it first as a separate thing.


As above. We're touching all this code anyway, lets clean it up and make it better while doing such changes. Otherwise we'll just have more stuff stacked ontop of this making it even harder to fix up and causing more big-ish breaks in the future.


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


Restrictions on the plugins should only be applied to preserve invariants
that Geany actually depends on, otherwise we are just pretending we can
anticipate all the uses a plugin might need.  And if as you say, one plugin
is already using those settings, then I guess we missed that use-case :)


It goes a long way to make an API consistent though, the mismatch between document_set_text_changed() and doc->changed from an API POV is not very nice, and being able to do `doc->changed = FALSE;` in a plugin might even be dangerous, if not only make the UI inconsistent with it's backing data structure.

Also, when you have something like document_get_text_changed(), you can later re-write to ask Scintilla if it's model has unsaved changes rather than synchronizing them manually. You can also do all kinds of other interesting stuff inside the "getters", but you can't do it later if you just expose direct access to the structure's members, as I'm sure you know.

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

Reply via email to