[patch] Fix some issues about the terminal feature

94 views
Skip to first unread message

Ken Takata

unread,
Jul 24, 2017, 8:35:07 AM7/24/17
to vim_dev
Hi,

I found some issues with the terminal feature (mainly on Windows):

* Couldn't build with VC2010, because libvterm requires stdbool.h.
Reuse if_perl_msvc/stdbool.h for this.
(Maybe it's better to change the directory name "if_perl_msvc", but I didn't
do it this time.)

* vterm.lib was not build automatically by Make_mvc.mak. Add it to the
dependency of vim.exe.
(src/Makefile doesn't build libvterm.a and compiles/links it directly.
Maybe it's better to do the same thing for src/Make_mvc.mak and
src/Make_cyg_ming.mak, because we cannot build both 32- and 64-bit vim.exe
in the same directory when the terminal feature is enabled.)

* Winpty didn't work with 32-bit vim.exe. Fix some function declarations.

* Fix crash when 'encoding' isn't "utf-8".

* Fix document.
- Fix typos.
- Move instructions for installing winpty to runtime/doc/terminal.txt from
src/INSTALLpc.txt
- etc.

Please check the attached patch.

Regards,
Ken Takata

terminal-vc2010.patch

Bram Moolenaar

unread,
Jul 24, 2017, 4:29:46 PM7/24/17
to vim...@googlegroups.com, Ken Takata

Ken Takata wrote:

> I found some issues with the terminal feature (mainly on Windows):
>
> * Couldn't build with VC2010, because libvterm requires stdbool.h.
> Reuse if_perl_msvc/stdbool.h for this.
> (Maybe it's better to change the directory name "if_perl_msvc", but I didn't
> do it this time.)

Only one file includes stdbool.h. It's easy enough to change true and
false to TRUE and FALSE, like we do in Vim.

> * vterm.lib was not build automatically by Make_mvc.mak. Add it to the
> dependency of vim.exe.
> (src/Makefile doesn't build libvterm.a and compiles/links it directly.
> Maybe it's better to do the same thing for src/Make_mvc.mak and
> src/Make_cyg_ming.mak, because we cannot build both 32- and 64-bit vim.exe
> in the same directory when the terminal feature is enabled.)
>
> * Winpty didn't work with 32-bit vim.exe. Fix some function declarations.
>
> * Fix crash when 'encoding' isn't "utf-8".
>
> * Fix document.
> - Fix typos.
> - Move instructions for installing winpty to runtime/doc/terminal.txt from
> src/INSTALLpc.txt
> - etc.
>
> Please check the attached patch.

Thanks!

--
hundred-and-one symptoms of being an internet addict:
235. You start naming your kids Pascal, COBOL, Algol and Fortran.

/// Bram Moolenaar -- Br...@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 ///

Ken Takata

unread,
Jul 25, 2017, 9:36:02 AM7/25/17
to vim_dev, ktakat...@gmail.com
Hi Bram,

2017/7/25 Tue 5:29:46 UTC+9 Bram Moolenaar wrote:
> Ken Takata wrote:
>
> > I found some issues with the terminal feature (mainly on Windows):
> >
> > * Couldn't build with VC2010, because libvterm requires stdbool.h.
> > Reuse if_perl_msvc/stdbool.h for this.
> > (Maybe it's better to change the directory name "if_perl_msvc", but I didn't
> > do it this time.)
>
> Only one file includes stdbool.h. It's easy enough to change true and
> false to TRUE and FALSE, like we do in Vim.

I think you edited a wrong file in the patch 8.0.0769.
pen.c should have been edited. Moreover, vterm.h incudes stdbool.h.
Attached patch fixes the problem.


> > * vterm.lib was not build automatically by Make_mvc.mak. Add it to the
> > dependency of vim.exe.
> > (src/Makefile doesn't build libvterm.a and compiles/links it directly.
> > Maybe it's better to do the same thing for src/Make_mvc.mak and
> > src/Make_cyg_ming.mak, because we cannot build both 32- and 64-bit vim.exe
> > in the same directory when the terminal feature is enabled.)

Attached patch also applies these changes.
Now we can build both 32- and 64-bit versions in the same directory.
(Actually, object files are stored in different directories, however the
executable files are written in the src/ directory, so they can be
overwritten.)

Regards,
Ken Takata

fix-terminal-win32.patch

Bram Moolenaar

unread,
Jul 25, 2017, 3:47:55 PM7/25/17
to vim...@googlegroups.com, Ken Takata

Ken Takata wrote:

> 2017/7/25 Tue 5:29:46 UTC+9 Bram Moolenaar wrote:
> > Ken Takata wrote:
> >
> > > I found some issues with the terminal feature (mainly on Windows):
> > >
> > > * Couldn't build with VC2010, because libvterm requires stdbool.h.
> > > Reuse if_perl_msvc/stdbool.h for this.
> > > (Maybe it's better to change the directory name "if_perl_msvc", but I didn't
> > > do it this time.)
> >
> > Only one file includes stdbool.h. It's easy enough to change true and
> > false to TRUE and FALSE, like we do in Vim.
>
> I think you edited a wrong file in the patch 8.0.0769.
> pen.c should have been edited. Moreover, vterm.h incudes stdbool.h.
> Attached patch fixes the problem.

Somehow my grep missed that. I'll make a separate patch for this.

> > > * vterm.lib was not build automatically by Make_mvc.mak. Add it to the
> > > dependency of vim.exe.
> > > (src/Makefile doesn't build libvterm.a and compiles/links it directly.
> > > Maybe it's better to do the same thing for src/Make_mvc.mak and
> > > src/Make_cyg_ming.mak, because we cannot build both 32- and 64-bit vim.exe
> > > in the same directory when the terminal feature is enabled.)
>
> Attached patch also applies these changes.
> Now we can build both 32- and 64-bit versions in the same directory.
> (Actually, object files are stored in different directories, however the
> executable files are written in the src/ directory, so they can be
> overwritten.)

I suppose it was getting too complicated to keep the Makefile under
libvterm. Oh well, we are not getting to have this included in the
libvterm distribution then.

There were a few merge conflicts, please check the patch.

--
If you're sending someone Styrofoam, what do you pack it in?

Ken Takata

unread,
Jul 25, 2017, 11:43:32 PM7/25/17
to vim_dev, ktakat...@gmail.com
Hi Bram,

Ah, sorry. I mistakenly mixed another patch to use -output option instead
of redirection when generating if_perl.c from if_perl.xs with xsubpp.

The -output option for xsubpp has been supported since Perl 5.10.
Using -output has a merit when xsubpp fails with some reasons. If we use
redirection, an empty if_perl.c will be generated, and it cause some weird
errors. However, if we use -output, if_perl.c will not be generated and we can
easily know that there are some errors in xsubpp.

The patch for Make_cyg_ming.mak was mistakenly included, but how about
including the following additional patch?

--- a/src/Make_mvc.mak
+++ b/src/Make_mvc.mak
@@ -1341,7 +1341,7 @@ testclean:

if_perl.c : if_perl.xs typemap
$(XSUBPP) -prototypes -typemap $(XSUBPP_TYPEMAP) \
- -typemap typemap if_perl.xs > if_perl.c
+ -typemap typemap if_perl.xs -output if_perl.c

$(OUTDIR)/if_perl.obj: $(OUTDIR) if_perl.c $(INCL)
$(CC) $(CFLAGS_OUTDIR) $(PERL_INC) if_perl.c


I don't think we should still support Perl 5.8 on Windows.
(I have heard that there are some Linux distributions which still support
Perl 5.8. So, this change might not be better to be applied to src/Makefile.)


If you don't like using -output, you can revert the change by applying the
following patch:

--- a/src/Make_cyg_ming.mak
+++ b/src/Make_cyg_ming.mak
@@ -951,7 +951,7 @@ mzscheme_base.c:

if_perl.c: if_perl.xs typemap
$(XSUBPP) -prototypes -typemap \
- $(PERLTYPEMAP) if_perl.xs -output $@
+ $(PERLTYPEMAP) if_perl.xs > $@

$(OUTDIR)/if_ruby.o: if_ruby.c $(INCL)
ifeq (16, $(RUBY))


Regards,
Ken Takata

Bram Moolenaar

unread,
Jul 26, 2017, 5:11:28 PM7/26/17
to vim...@googlegroups.com, Ken Takata
OK, let's update Make_mvc.mak and await if some people have a problem
with that.

--
hundred-and-one symptoms of being an internet addict:
261. You find diskettes in your pockets when doing laundry.
Reply all
Reply to author
Forward
0 new messages