Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#16 GTK3

None
closed
None
5
2014-08-25
2011-05-06
John Ralls
No

There's a super-simple patch to make GtkSpell work with Gtk3 which was submitted to RedHat [1] in February. Perhaps someone could commit it here and make a new tarball...

[1] https://bugzilla.redhat.com/show_bug.cgi?id=675504

Discussion

1 2 3 .. 5 > >> (Page 1 of 5)
  • John Ralls
    John Ralls
    2011-05-06

     
    Attachments
  • I have a patch against cvs to create a 3.0.0 tree, which installs libgtkspell-3.0.so and gtkspell-3.0.pc, so it can be installed in parallel with gtkspell-2.0. If I could figure out how to upload an attachment to this bug, I'd upload it (and the required gtkspell-3.0.pc.in)...

     

  • Anonymous
    2011-07-25

    I have a modified version of the patch which makes it installable side-by-side with the gtk2 built version. I have uploaded this patch to ubuntu already.

    I couldn't figure out how to add it as an attachment, you can download it from:

    http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/oneiric/gtkspell/oneiric/view/head:/debian/patches/gtk3_support.patch

     
  • Daniel Atallah
    Daniel Atallah
    2011-08-16

    A change that's a combination of these patches has been committed and will be released as Gtkspell 3.0.0 soon.

    Thanks!

     
  • Hi Daniel,

    I didn't see a release yet--did I miss something?

    Peter

     
  • Could someone please reopen this bug? It really shouldn't be closed until the tarball requested by the OP is released...

     
  • Sandro Mani
    Sandro Mani
    2012-01-01

    So, needing gtkspell3 to finish porting my PyGtk to Gtk3, I've worked on adding introspection annotations to the test tarball. Result is here:
    http://n.ethz.ch/~smani/download/gtkspell3-3.0.test20120101-0.1.fc16.src.rpm

    Note: From what I could tell, in the previous version, Gtkspell was not actually a GObject type. For this reason, I had to perform a number of modifications to the source code, leading also to API changes. The most prominent changes are
    - All functions now start with gtkspell_spell_ instead of gtkspell_. This is because gtkspell is now the namespace and spell the class (i.e. GtkspellSpell).
    - The gtkspell_spell_new_attach function is gone, replaced by gtkspell_spell_new and gtkspell_spell_attach (which were actually deprecated functions). This is because the API allows to detach a GtkspellSpell object from a TextView without actually destroying the spell object, so it made more sense to also separate creation and attachment.

    Feedback / reaction from the OP would be great.

     
  • John Ralls
    John Ralls
    2012-01-02

    My first comment is "I don't do rpms", and my second is "You expect me to diff the package?"

    The best way to get a review would be to start a repo on github with the current cvs snapshot, then commit your changes in an easy-to-follow series with good change messages.

     
  • John Ralls
    John Ralls
    2012-01-02

    Oops, looks like Daniel has switched to mercurial. That will in any case make it easier for you to make your Github branch (or you could use Bitbucket).

    The new name, though ugly, is an unfortunate necessity of GObject Introspection, and I certainly agree that constructors (and destructors) need to be focused on
    construction (or destruction).

     
  • John Ralls
    John Ralls
    2012-01-02

    You went a bit further than just fixing it up for gobject-introspection. What's your use-case for the LANGUAGE_CHANGED signal?

    There are a lot of coding style errors (but there are in the original, too): http://git.gnome.org/browse/gtk+/plain/docs/CODING-STYLE

    Other than that, it looks fine... but I haven't tried to build it, nor do I have a good gtk3 test bed for it.

     
  • Sandro Mani
    Sandro Mani
    2012-01-02

    So concerning the signal: I had the problem that I had another control for setting the spellcheck language, and if the user would then modify if from the menu, the two would get out of sync.

    If you find it necessary I'll fix up all the coding style related stuff.

    Concerning compiling: compiles fine without warnings, the only problem is that gtk-doc does not recognize the annotation tags. The fix is mentioned here http://live.gnome.org/GObjectIntrospection/Annotations (all the way down), but as soon as I add the mentioned include, I get all sorts parser errors, I guess because it's a sgml file and not an xml. Maybe the docs should be ported to XML?

    Usage: works as expected afaict, using it through PyGi atm. Feedback from here is also positive: https://bugzilla.redhat.com/show_bug.cgi?id=675504#c10

     
  • John Ralls
    John Ralls
    2012-01-02

    Um, I'm the original poster, not the maintainer... but I'm a Gtk-quartz maintainer, so I've gotten sensitized to the coding standards (meaning I've been whacked often with a rolled up copy ;-) ). It's up to Daniel Atallah, the current project owner here, to decide whether you should fix up the coding style to match Gnome's (and whether he should, too ;-) ).

    I don't know if he's still monitoring this, having closed it; if he doesn't respond in a few days you should open a new ticket and attach a set of patches... but I'd break up the 77403c20 commit to separate making the GtkspellSpell GObject, the introspection annotations, the new signal, and any other added features that I missed (a separate patch for each) -- and leaving out any gratuitous reformatting. (If you were submitting this to Gtk we'd want a separate bug report for each of those patches.)

     
  • Sandro Mani
    Sandro Mani
    2012-01-02

    Uhm ok, thanks for the review in the meantime!

     
  • Daniel Atallah
    Daniel Atallah
    2012-01-02

    I really need to apologize for being extremely unavailable.

    Unfortunately, my time for side projects has been practically non-existent the past few months.

    There hasn't been a release of the currently committed 3.0.0 changes (and that probably is a good thing because it makes it possible to do this sort of thing without a further version bump).

    The GObject-ification is something that I've really been wanting to do, so I'm really happy to see a patch that does that.

    As far as the coding style goes - this is old code and very little of it is actually mine. I certainly agree that isn't the most consistent. I'm not necessarily a huge fan of the gnome style, but I think that consistency is more important than anything else (within reason) where coding style is concerned. I'd be happy if new stuff were to be consistent, but I don't think it is a strict prerequisite to accepting a patch.

    Now for some comments about the actual changes:

    Good job removing the deprecated functions - I should have already done that.
    I'm not super happy about the function name changes, but if that is what's needed to make this work, it isn't the end of the world.
    The ability to tell when a language is changed is a good thing - I opened a ticket for that a while ago [1]

    I wonder if some variation of this (without the API changes) could be adapted to the GTK2 branch - started down this path a while ago and didn't have time to finish. If so, I would rather this be done like that (and then the API changes be made to the 3.0.0 branch).
    Do you think that is reasonably doable?

    [1] https://sourceforge.net/tracker/?func=detail&aid=3015525&group_id=7896&atid=357896

     
  • Daniel Atallah
    Daniel Atallah
    2012-01-02

    I do agree with John's sentiment of separate patches / commits for separate features, but I don't think it needs to be a separate ticket (this is such a low activity project that it isn't really necessary).

     
  • John Ralls
    John Ralls
    2012-01-02

    The usual pattern for broadly applicable major changes is to do them on the development branch ("default" in Hg) and then backport them to stable branches.

    GSignals are part of GObject, so you'd need to backport the GObjectification as well. You could just call the object GtkSpell in Gtk2, there's not much point in using GObject-Introspection there. That would let you keep mostly the same API, you'd just make gtkspell_new_attach() a convenience function that calls gtkspell_new() (itself a convenience function for g_object_new (GTKSPELL_TYPE) ).

    As for coding style, I'm not fond of parts of it either, but ISTM if a project is named "gtkfoo" or "gnomefoo" it ought to follow the Gnome style.

     
  • Sandro Mani
    Sandro Mani
    2012-01-03

    Ok, so i splitted up the changes into small patches, work is in the new branch "changes", here
    https://github.com/manisandro/gtkspell3/tree/changes

    I could also give the gtk2 version a shot, i.e. apply b0c5359e32242b263c637ec6266a0dd4ee349c71 to the gtk2 branch with some changes (gtkspell_spell -> gtkspell, gtkspell_new_attach as a convenience function).

    Feedback welcome.

     
  • Sandro Mani
    Sandro Mani
    2012-01-19

    Any news on this?

     
  • Daniel Atallah
    Daniel Atallah
    2012-01-23

    I guess I was waiting for the gtk2 branch changes.

    I think it makes more sense to make those in the gtk2 branch, merge them up to the gtk3 branch and make the additional cleanup changes there.

    Trying to backport part of another set of changes is a recipe for merge nightmare.

     
  • Sandro Mani
    Sandro Mani
    2012-01-23

    Ok, I'll work on that, will report back as soon as I'm done - hope by next week.

     
  • Sandro Mani
    Sandro Mani
    2012-02-06

    Ok, here we go: https://github.com/manisandro/gtkspell2 (branch changes). I'll recreate the gtkspell3 patches once the gtkspell2 changes are accepted into master.

     
  • Daniel Atallah
    Daniel Atallah
    2012-02-06

    Sandro, thanks for the patches.

    I've reviewed and here are some notes, comments and questions:

    There must be no API removals or ABI changes in the gtkspell2 branch - people who previously compiled against 2.0.16 need to be able to run with (and compile against) 2.0.17 (or 2.1.0 - whatever this gets named). This means that https://github.com/manisandro/gtkspell2/commit/0cfcbf3dd92e3d262db2ee9b9e77d8cb7b2e736f will need to wait for the gtkspell3 branch.

    What is the deal with the removal of "docs/tmpl"?

    I'm not sure about the reformatting commit.
    It's certainly good it is done separately than the rest (although there appear to also be some formatting changes in d46cb5f662efda560ddeda686bd4da243839f1bd), but I'm not really convinced that it's really worthwhile to change all the existing code just for formatting reasons.
    Was this done manually or did you use a utility to apply the formatting?

    The rest of this seems reasonable and safe to include in gtkspell2 - I still need to compile and do a little testing though.

     
  • Sandro Mani
    Sandro Mani
    2012-02-06

    Hello Daniel,

    Deprecated:
    - The gtkspell_attach function exists again and is valid, so as far as that function is concerned there is no problem.
    - For the gtkspell_init on the other hand, there is a problem: the GObject system forces me to create a gtkspell_init function and to call it that way. While the old deprecated gtkspell_init has a different signature, this does not help since C does not support function overloading... Hence from what I can see, an API break is unavoidable, but at least the user gets a compilation error (because of the signature mismatch), and therefore the case where gtkspell_init gets called twice (once because of the GObject init, once because of the call in the user code) is avoided.

    docs/tmpl:
    From what I can tell they were simply the autogenerated files which were added to version control. Additionally, I got some compilation warnings (i.e.
    "../gtkspell/gtkspell.h:21: warning: Documentation in template ./tmpl/gtkspell.sgml:114 for GTKSPELL_ERROR being overridden by inline comments.", and similar for GtkSpellError and GtkSpell). For that reason, I think it is best to simply let gtk-doc autogenerate them always.

    Refromatting: it was done with a combination of regular expressions. There are indeed some formatting changes in d46cb5f662efda560ddeda686bd4da243839f1bd, essentially because I copied the functions from the gtkspell3 branch and did the GtkspellSpell -> GtkSpell renaming

     
  • Daniel Atallah
    Daniel Atallah
    2012-02-06

    Hmm... This is a problem. Unless there is a way to avoid the API and ABI breakage, it isn't going to be possible to include this in the gtkspell2 branch.

    I think it should be possible to keep the function in deprecated.c as it is now and to simply define GTKSPELL_DISABLE_DEPRECATED at the top of gtkspell.c before gtkspell.h is included to avoid a naming conflict. The new function is static in gtkspell.c and not part of any exposed API.

    I'm not familiar with the origins of the docs/tmpl files, as long as the doc generation still works and looks the same without them, I'm fine killing them.

     
1 2 3 .. 5 > >> (Page 1 of 5)