Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GeanyVC] use compatible gtkspell on building for gtk3 #342

Closed
wants to merge 4 commits into from

Conversation

sagarchalise
Copy link
Contributor

When building against gtk3 the gtkspell support was missing for geanyvc plugin commit dialog box.

@sagarchalise sagarchalise changed the title use compatible gtkspell on building for gtk3 [GeanyVC] use compatible gtkspell on building for gtk3 Jan 27, 2016
@frlan
Copy link
Member

frlan commented Feb 12, 2016

I'm afraid build is failing here


geanyvc.c: In function 'vccommit_activated':
geanyvc.c:1536:2: error: unknown type name 'GtkSpellChecker'
  GtkSpellChecker *speller = NULL;
  ^
geanyvc.c:1598:17: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]
         speller = gtkspell_new_attach(GTK_TEXT_VIEW(messageView), NULL, &spellcheck_error);
                 ^
geanyvc.c:1612:34: warning: passing argument 1 of 'gtkspell_set_language' from incompatible pointer type [-Wincompatible-pointer-types]
   gtk_spell_checker_set_language(speller, lang, &spellcheck_error);
                                  ^
In file included from geanyvc.c:41:0:
/usr/include/gtkspell-2.0/gtkspell/gtkspell.h:34:11: note: expected 'GtkSpell * {aka struct _GtkSpell *}' but argument is of type 'int *'
 gboolean  gtkspell_set_language(GtkSpell *spell,
           ^

@b4n
Copy link
Member

b4n commented Feb 13, 2016

Works for me after the mentioned fix.

@@ -1528,7 +1528,12 @@ vccommit_activated(G_GNUC_UNUSED GtkMenuItem * menuitem, G_GNUC_UNUSED gpointer
gint height;

#ifdef USE_GTKSPELL
GtkSpell *speller = NULL;
#if !GTK_CHECK_VERSION (3, 0, 0)
//Since gtk3 will be default in future may be going this way would be better.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment is not very useful, but I agree adding compat layer for older version rather than newer

@b4n
Copy link
Member

b4n commented Feb 13, 2016

If we wanted a simpler/less intrusive port, we could just do

diff --git a/geanyvc/src/geanyvc.c b/geanyvc/src/geanyvc.c
index a95a991..488f1f2 100644
--- a/geanyvc/src/geanyvc.c
+++ b/geanyvc/src/geanyvc.c
@@ -39,6 +39,25 @@

 #ifdef USE_GTKSPELL
 #include <gtkspell/gtkspell.h>
+/* forward compatibility with GtkSpell3 */
+#      if GTK_CHECK_VERSION(3, 0, 0)
+#              define GtkSpell GtkSpellChecker
+#              define gtkspell_set_language gtk_spell_checker_set_language
+static GtkSpell *gtkspell_new_attach(GtkTextView *view, const gchar *lang, GError **error)
+{
+       GtkSpellChecker *speller = gtk_spell_checker_new();
+
+       if (! lang || gtk_spell_checker_set_language(speller, lang, error))
+               gtk_spell_checker_attach(speller, view);
+       else
+       {
+               g_object_unref(g_object_ref_sink(speller));
+               speller = NULL;
+       }
+
+       return speller;
+}
+#      endif
 #endif

 GeanyData *geany_data;

in the code, and just keep the build system changes from this PR, which are just fine.

[edit: fixed unref with regard to floating ref]

@sagarchalise
Copy link
Contributor Author

Sorry about the typo. Have made changes according to suggestions made by @b4n .

@b4n
Copy link
Member

b4n commented Feb 17, 2016

Indentation style is incorrect in geanyvc.c (should use tabs (with width=4), like the rest of the file). Commits should be squashed.
I can do both when merging this if you like.

Apart that, looks good and seems to work fine on both GTK2 and 3. BTW, good job simplifying the error handling that indeed was unnecessarily complex :)

@sagarchalise
Copy link
Contributor Author

@b4n Do whatever you consider necessary. Also, I guess the error message should have something about GeanyVC as user may misunderstand it to be message of spell check plugin. Something like

  • Error initializing spell checking for geanyvc:
    or
  • [GeanyVC] Error initializing spell checking:

@b4n b4n closed this in 57e0cd0 Feb 17, 2016
@b4n
Copy link
Member

b4n commented Feb 17, 2016

@b4n Do whatever you consider necessary.

Okay, done, committed as 57e0cd0.

Also, I guess the error message should have something about GeanyVC as user may misunderstand it to be message of spell check plugin. […]

Good point. Done in a788c31

@b4n b4n added this to the 1.27 milestone Feb 17, 2016
@b4n b4n self-assigned this Feb 17, 2016
@b4n b4n removed the pending fixes label Feb 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants