search

vim / vim

The official Vim repository
https://www.vim.org
Vim License
36.04k stars 5.38k forks source link

Have vim use hunspell and hunspell dicts directly #846

Open sanjayankur31 opened 8 years ago

sanjayankur31 commented 8 years ago

From this inactive bug. A patch is attached there already and was sent to the vim devel list. It needs some work.

I'm just filing the issue so that someone with the skills and time may take it up.

Mailing list RFC: http://www.mail-archive.com/vim-dev@vim.org/msg03252.html

mcepl commented 8 years ago

It may be helpful to add the patch here https://gist.github.com/mcepl/f933a53de7e14909bded64afad6754b7

And also to note that the patch is originally by @caolanm

brammool commented 8 years ago

Matěj Cepl wrote:

It may be helpful to add the patch here https://gist.github.com/mcepl/f933a53de7e14909bded64afad6754b7

Unfortunately that patch is unfinished. It needs a configure part and support for non-Unix systems. And the code formatting needs to be improved (e.g., not use // comments).

It is hard to understand how a cemetery raised its burial cost and blamed it on the cost of living.

/// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \ \ an exciting new programming language -- http://www.Zimbu.org /// \ help me help AIDS victims -- http://ICCF-Holland.org ///

mcepl commented 8 years ago

Unfortunately that patch is unfinished.

Yes, I know that. That was the whole point of that Fedora bug. And the reason for this ticket is a frustration that there is no programmer using vim everyday, who would be willing to fix that patch and make it acceptable.

mcepl commented 7 years ago

Refreshed patch at https://gist.github.com/mcepl/190c8d7fabafd142928dc41b5c441f87 applies against 8.0.442.

chrisbra commented 7 years ago

That patch still needs changes to the configure part.

mcepl commented 6 years ago

Yeah, unfortunately (to all those who would think, that fixing configure.ac is easy), vim doesn't use autofoo properly, so changing it seems to the idiot like me impossible. Even what seems like a trivial change leads to the endless NIH swamp of proprietary extensions to autofoo (e.g., autoreconf doesn't work at all):

--- /home/matej/vim-7.0-hunspell.patch  2017-12-17 00:52:01.884214440 +0100
+++ vim-8.0-hunspell.patch  2017-12-17 00:26:38.135378188 +0100
@@ -1,19 +1,17 @@
-diff -up vim-8.0.0363/src/Makefile.hunspell vim-8.0.0363/src/Makefile
---- vim-8.0.0363/src/Makefile.hunspell 2017-02-23 20:20:53.000000000 +0100
-+++ vim-8.0.0363/src/Makefile  2017-02-24 14:29:28.614983465 +0100
-@@ -1446,7 +1446,8 @@ ALL_LIBS = \
+--- a/src/Makefile
++++ b/src/Makefile
+@@ -1473,7 +1473,8 @@ ALL_LIBS = \
       $(RUBY_LIBS) \
       $(PROFILE_LIBS) \
       $(SANITIZER_LIBS) \
 -     $(LEAK_LIBS)
 +     $(LEAK_LIBS) \
-+     -lhunspell-1.4
++     $(HUNSPELL_LIBS)

  # abbreviations
  DEST_BIN = $(DESTDIR)$(BINDIR)
-diff -up vim-8.0.0363/src/spell.c.hunspell vim-8.0.0363/src/spell.c
---- vim-8.0.0363/src/spell.c.hunspell  2017-02-23 20:20:53.000000000 +0100
-+++ vim-8.0.0363/src/spell.c   2017-02-24 14:30:28.211114737 +0100
+--- a/src/spell.c
++++ b/src/spell.c
 @@ -405,6 +405,36 @@ static linenr_T dump_prefixes(slang_T *s
  static char_u *repl_from = NULL;
  static char_u *repl_to = NULL;
@@ -90,7 +88,7 @@
 +
 +      do
 +      {
-+          mb_ptr_adv(mi_end);
++          MB_PTR_ADV(mi_end);
 +      } while (*mi_end != NUL && spell_iswordp(mi_end, wp->w_buffer));
 +      }
 +  }
@@ -276,7 +274,7 @@
      }

      /* round 0: load int_wordlist, if possible.
-@@ -4295,6 +4458,36 @@ suggest_try_change(suginfo_T *su)
+@@ -4297,6 +4460,36 @@ suggest_try_change(suginfo_T *su)
      {
    lp = LANGP_ENTRY(curwin->w_s->b_langp, lpi);

@@ -313,9 +311,8 @@
    /* If reloading a spell file fails it's still in the list but
     * everything has been cleared. */
    if (lp->lp_slang->sl_fbyts == NULL)
-diff -up vim-8.0.0363/src/spellfile.c.hunspell vim-8.0.0363/src/spellfile.c
---- vim-8.0.0363/src/spellfile.c.hunspell  2017-02-24 14:30:10.618075985 +0100
-+++ vim-8.0.0363/src/spellfile.c   2017-02-24 14:30:28.211114737 +0100
+--- a/src/spellfile.c
++++ b/src/spellfile.c
 @@ -832,7 +832,7 @@ read_region_section(FILE *fd, slang_T *l
  {
      int       i;
@@ -363,9 +360,8 @@
      else
      {
    /* Check for overwriting before doing things that may take a lot of
-diff -up vim-8.0.0363/src/spell.h.hunspell vim-8.0.0363/src/spell.h
---- vim-8.0.0363/src/spell.h.hunspell  2017-02-24 14:30:16.573089102 +0100
-+++ vim-8.0.0363/src/spell.h   2017-02-24 14:30:28.211114737 +0100
+--- a/src/spell.h
++++ b/src/spell.h
 @@ -29,10 +29,14 @@
  # define DEBUG_TRIEWALK
  #endif
@@ -401,3 +397,29 @@

      char_u    *sl_midword;    /* MIDWORD string or NULL */

+--- a/src/configure.ac
++++ b/src/configure.ac
+@@ -3080,6 +3080,23 @@ else
+   AC_MSG_RESULT(no)
+ fi
+ 
++dnl ---------------------------------------------------------------------------
++dnl Using hunspell
++dnl ---------------------------------------------------------------------------
++AC_MSG_CHECKING(--with-hunspell argument)
++AC_ARG_WITH(hunspell,
++            AS_HELP_STRING([--without-hunspell],
++                           [ignore presence of hunspell and disable it]))
++AS_IF([test "x$with_hunspell" != "xno"],
++      [
++       AC_MSG_RESULT($with_hunspell)
++       PKG_CHECK_MODULES([HUNSPELL], [hunspell], [have_hunspell=yes], [have_hunspell=no])
++      ],
++      [have_hunspell=no])
++AC_SUBST(HUNSPELL_CFLAGS)
++AC_SUBST(HUNSPELL_LIBS)
++
++
+ dnl Check for Cygwin, which needs an extra source file if not using X11
+ AC_MSG_CHECKING(for CYGWIN or MSYS environment)
+ case `uname` in
k-takata commented 6 years ago

If you change src/configure.ac, you should run make autoconf to update src/auto/configure.

mcepl commented 6 years ago

Which I certainly did, but src/auto/configure is not changed accordingly. In the end I rather do

HUNSPELL_LIBS="$(pkg-config --libs-only-l hunspell)"
sed -i -e "s/##HUNSPELL_LIBS/$HUNSPELL_LIBS/" src/Makefile

which is at least reliable.

mcepl commented 6 years ago

With a very kind help of @dtardon I have finally managed to fix the patch to work properly. The last build has this new patch.

mcepl commented 6 years ago

@brammool Could I get please some summary of what's wrong with this patch as it is now? I am not sure what should I do about Windows (I don't have them anywhere around and I know almost nothing about the Windows programming).

mcepl commented 6 years ago

Should I make a proper pull request?

robert-scheck commented 6 years ago

At @@ -1952,7 +1952,7 @@ typedef struct spellinfo_S, I found:

-    char_u si_region_name[17]; /* region names; used only if
+    char_u si_region_name[MAXREGIONS*2]; /* region names; used only if

Shouldn't that be si_region_name[MAXREGIONS * 2 + 1] instead? This is at least how it was done at another place in the patch.

mcepl commented 6 years ago

OK, so I have created #2500 to make the discussion pointed towards the solution. I am not sure whether it is custom to mark pull requests in progress as WIP, but that's what I mean. Please, review and comment.