Loading ...
Sorry, an error occurred while loading the content.
 

Re: MacVim + Ruby 1.9.1

Expand Messages
  • Michael Shapiro
    ... I pulled the vim 7.2 source from http://www.vim.org/sources.php and poked around for ruby config arguments. The provide your own ruby option is missing.
    Message 1 of 16 , May 12 12:15 PM
      On May 12, 2010, at 1:39 PM, björn wrote:

      > Should this patch be made against mainline Vim instead of MacVim (i.e.
      > is there any other platform that might benefit from it)? If so, could
      > you please create a new patch and send it on to vim_dev? Otherwise
      > I'll merge it with the MacVim repo.

      I pulled the vim 7.2 source from http://www.vim.org/sources.php and poked around for ruby config arguments. The 'provide your own ruby' option is missing.

      vim72 $ ./configure --help | grep ruby
      --enable-rubyinterp Include Ruby interpreter.

      MacVim $ ./configure --help | grep ruby
      --enable-rubyinterp Include Ruby interpreter.
      --with-ruby-command=RUBY name of the Ruby command (default: ruby)

      From the OS X-specific nature of the --with-ruby-command option in configure.in, it looks like it wasn't written with other platforms in mind. If it were to be submitted to vim_dev, it should probably be generalized for linux and windows.

      >
      > I took a quick look at the patch but it is not entirely clear to me
      > what it does? Is it only vi_cv_path_ruby that is set earlier or is
      > something else going on?

      Sorry about the messy patch, I was correcting the indenting at the same time.

      Here's a far better patch. http://gist.github.com/398996

      The problems were:
      1. The original --with-ruby-command assumed that the provided ruby was in the users PATH, and not an absolute path to a ruby binary. (AC_PATH_PROG by default looks in the user's PATH). I've corrected this by providing the path to the binary to AC_PATH_PROG.

      2. If a custom ruby is provided, we don't want to use -framework Ruby. There's a conditional to check for that now.

      3. The bit of ruby used as a fallback to provide library directories assumed that "." would be one of them. I've corrected that.

      The embedded conditional does some checks for ruby on previous versions of OS X, I don't know enough about the state of old versions, so I just left it as the default behavior if an absolute-pathed ruby isn't provided.

      Hope that clear things up a bit. :)

      --Mike

      >
      > Björn
      >
      > --
      > You received this message from the "vim_mac" maillist.
      > Do not top-post! Type your reply below the text you are replying to.
      > For more information, visit http://www.vim.org/maillist.php

      --
      You received this message from the "vim_mac" maillist.
      Do not top-post! Type your reply below the text you are replying to.
      For more information, visit http://www.vim.org/maillist.php
    • björn
      ... Mike, could you do me a favor and update this patch against the latest source code version [1] and repost it. Also, please use git-format-patch so that
      Message 2 of 16 , Jul 26, 2010
        On 12 May 2010 21:15, Michael Shapiro wrote:
        > On May 12, 2010, at 1:39 PM, björn wrote:
        >
        >> Should this patch be made against mainline Vim instead of MacVim (i.e.
        >> is there any other platform that might benefit from it)?  If so, could
        >> you please create a new patch and send it on to vim_dev?  Otherwise
        >> I'll merge it with the MacVim repo.
        >
        > I pulled the vim 7.2 source from http://www.vim.org/sources.php and poked around for ruby config arguments. The 'provide your own ruby' option is missing.
        >
        > vim72 $ ./configure --help | grep ruby
        >  --enable-rubyinterp     Include Ruby interpreter.
        >
        > MacVim $ ./configure --help | grep ruby
        >  --enable-rubyinterp     Include Ruby interpreter.
        >  --with-ruby-command=RUBY  name of the Ruby command (default: ruby)
        >
        > From the OS X-specific nature of the --with-ruby-command option in configure.in, it looks like it wasn't written with other platforms in mind. If it were to be submitted to vim_dev, it should probably be generalized for linux and windows.
        >
        >>
        >> I took a quick look at the patch but it is not entirely clear to me
        >> what it does?  Is it only vi_cv_path_ruby that is set earlier or is
        >> something else going on?
        >
        > Sorry about the messy patch, I was correcting the indenting at the same time.
        >
        > Here's a far better patch. http://gist.github.com/398996
        >
        > The problems were:
        > 1. The original --with-ruby-command assumed that the provided ruby was in the users PATH, and not an absolute path to a ruby binary. (AC_PATH_PROG by default looks in the user's PATH). I've corrected this by providing the path to the binary to AC_PATH_PROG.
        >
        > 2. If a custom ruby is provided, we don't want to use -framework Ruby. There's a conditional to check for that now.
        >
        > 3. The bit of ruby used as a fallback to provide library directories assumed that "." would be one of them. I've corrected that.
        >
        > The embedded conditional does some checks for ruby on previous versions of OS X, I don't know enough about the state of old versions, so I just left it as the default behavior if an absolute-pathed ruby isn't provided.
        >
        > Hope that clear things up a bit. :)

        Mike, could you do me a favor and update this patch against the latest
        source code version [1] and repost it. Also, please use
        git-format-patch so that you get listed as the author of the patch.
        As soon as you send me an updated patch I'll take a look at it and
        merge asap.

        Sorry for taking so long to respond to this patch!

        Thanks,
        Björn


        [1] http://github.com/b4winckler/macvim

        --
        You received this message from the "vim_mac" maillist.
        Do not top-post! Type your reply below the text you are replying to.
        For more information, visit http://www.vim.org/maillist.php
      • Michael Shapiro
        ... You got it, dude. Tested on 10.6.4 with ruby 1.9.1-p378 built by RVM. I forked the repo, if you d prefer just a pull request. (
        Message 3 of 16 , Jul 26, 2010
          On Jul 26, 2010, at 2:31 PM, björn wrote:

          > On 12 May 2010 21:15, Michael Shapiro wrote:
          >> On May 12, 2010, at 1:39 PM, björn wrote:
          >>
          >>> Should this patch be made against mainline Vim instead of MacVim (i.e.
          >>> is there any other platform that might benefit from it)? If so, could
          >>> you please create a new patch and send it on to vim_dev? Otherwise
          >>> I'll merge it with the MacVim repo.
          >>
          >> I pulled the vim 7.2 source from http://www.vim.org/sources.php and poked around for ruby config arguments. The 'provide your own ruby' option is missing.
          >>
          >> vim72 $ ./configure --help | grep ruby
          >> --enable-rubyinterp Include Ruby interpreter.
          >>
          >> MacVim $ ./configure --help | grep ruby
          >> --enable-rubyinterp Include Ruby interpreter.
          >> --with-ruby-command=RUBY name of the Ruby command (default: ruby)
          >>
          >> From the OS X-specific nature of the --with-ruby-command option in configure.in, it looks like it wasn't written with other platforms in mind. If it were to be submitted to vim_dev, it should probably be generalized for linux and windows.
          >>
          >>>
          >>> I took a quick look at the patch but it is not entirely clear to me
          >>> what it does? Is it only vi_cv_path_ruby that is set earlier or is
          >>> something else going on?
          >>
          >> Sorry about the messy patch, I was correcting the indenting at the same time.
          >>
          >> Here's a far better patch. http://gist.github.com/398996
          >>
          >> The problems were:
          >> 1. The original --with-ruby-command assumed that the provided ruby was in the users PATH, and not an absolute path to a ruby binary. (AC_PATH_PROG by default looks in the user's PATH). I've corrected this by providing the path to the binary to AC_PATH_PROG.
          >>
          >> 2. If a custom ruby is provided, we don't want to use -framework Ruby. There's a conditional to check for that now.
          >>
          >> 3. The bit of ruby used as a fallback to provide library directories assumed that "." would be one of them. I've corrected that.
          >>
          >> The embedded conditional does some checks for ruby on previous versions of OS X, I don't know enough about the state of old versions, so I just left it as the default behavior if an absolute-pathed ruby isn't provided.
          >>
          >> Hope that clear things up a bit. :)
          >
          > Mike, could you do me a favor and update this patch against the latest
          > source code version [1] and repost it. Also, please use
          > git-format-patch so that you get listed as the author of the patch.
          > As soon as you send me an updated patch I'll take a look at it and
          > merge asap.
          >
          > Sorry for taking so long to respond to this patch!
          >
          > Thanks,
          > Björn
          >
          >
          > [1] http://github.com/b4winckler/macvim

          You got it, dude.

          Tested on 10.6.4 with ruby 1.9.1-p378 built by RVM.

          I forked the repo, if you'd prefer just a pull request. ( http://github.com/koudelka/macvim/commit/e550f312ee25696c4245fb9e2b58bfb0c78da05e )

          --Mike

          --
          You received this message from the "vim_mac" maillist.
          Do not top-post! Type your reply below the text you are replying to.
          For more information, visit http://www.vim.org/maillist.php
        • björn
          ... Thanks, but there is a problem: checking --enable-rubyinterp argument... yes checking --with-ruby-command argument... defaulting to ruby ./configure: line
          Message 4 of 16 , Jul 26, 2010
            On 26 July 2010 21:14, Michael Shapiro wrote:
            > On Jul 26, 2010, at 2:31 PM, björn wrote:
            >
            >> On 12 May 2010 21:15, Michael Shapiro wrote:
            >>> On May 12, 2010, at 1:39 PM, björn wrote:
            >>>
            >>>> Should this patch be made against mainline Vim instead of MacVim (i.e.
            >>>> is there any other platform that might benefit from it)?  If so, could
            >>>> you please create a new patch and send it on to vim_dev?  Otherwise
            >>>> I'll merge it with the MacVim repo.
            >>>
            >>> I pulled the vim 7.2 source from http://www.vim.org/sources.php and poked around for ruby config arguments. The 'provide your own ruby' option is missing.
            >>>
            >>> vim72 $ ./configure --help | grep ruby
            >>>  --enable-rubyinterp     Include Ruby interpreter.
            >>>
            >>> MacVim $ ./configure --help | grep ruby
            >>>  --enable-rubyinterp     Include Ruby interpreter.
            >>>  --with-ruby-command=RUBY  name of the Ruby command (default: ruby)
            >>>
            >>> From the OS X-specific nature of the --with-ruby-command option in configure.in, it looks like it wasn't written with other platforms in mind. If it were to be submitted to vim_dev, it should probably be generalized for linux and windows.
            >>>
            >>>>
            >>>> I took a quick look at the patch but it is not entirely clear to me
            >>>> what it does?  Is it only vi_cv_path_ruby that is set earlier or is
            >>>> something else going on?
            >>>
            >>> Sorry about the messy patch, I was correcting the indenting at the same time.
            >>>
            >>> Here's a far better patch. http://gist.github.com/398996
            >>>
            >>> The problems were:
            >>> 1. The original --with-ruby-command assumed that the provided ruby was in the users PATH, and not an absolute path to a ruby binary. (AC_PATH_PROG by default looks in the user's PATH). I've corrected this by providing the path to the binary to AC_PATH_PROG.
            >>>
            >>> 2. If a custom ruby is provided, we don't want to use -framework Ruby. There's a conditional to check for that now.
            >>>
            >>> 3. The bit of ruby used as a fallback to provide library directories assumed that "." would be one of them. I've corrected that.
            >>>
            >>> The embedded conditional does some checks for ruby on previous versions of OS X, I don't know enough about the state of old versions, so I just left it as the default behavior if an absolute-pathed ruby isn't provided.
            >>>
            >>> Hope that clear things up a bit. :)
            >>
            >> Mike, could you do me a favor and update this patch against the latest
            >> source code version [1] and repost it.  Also, please use
            >> git-format-patch so that you get listed as the author of the patch.
            >> As soon as you send me an updated patch I'll take a look at it and
            >> merge asap.
            >>
            >> Sorry for taking so long to respond to this patch!
            >>
            >> Thanks,
            >> Björn
            >>
            >>
            >> [1] http://github.com/b4winckler/macvim
            >
            > You got it, dude.
            >
            > Tested on 10.6.4 with ruby 1.9.1-p378 built by RVM.

            Thanks, but there is a problem:

            checking --enable-rubyinterp argument... yes
            checking --with-ruby-command argument... defaulting to ruby
            ./configure: line 5780: test: !=: unary operator expected
            checking for ruby... /usr/bin/ruby
            checking Ruby version... OK
            checking Ruby header files...
            /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/universal-darwin10.0

            Line 5780 comes from the following fragment in configure.in:

            if test $RUBY_PATH != $RUBY_CMD; then

            Can you fix this and resend the patch? Also, be careful with
            indentation, make sure you have "set noet" (some lines were not
            indented correctly).

            Thanks,
            Björn

            --
            You received this message from the "vim_mac" maillist.
            Do not top-post! Type your reply below the text you are replying to.
            For more information, visit http://www.vim.org/maillist.php
          • Michael Shapiro
            ... Hm, Sorry about that. I ve fixed the indenting and removed the need for that conditional completely. --Mike -- You received this message from the vim_mac
            Message 5 of 16 , Jul 26, 2010
              On Jul 26, 2010, at 5:31 PM, björn wrote:

              > On 26 July 2010 21:14, Michael Shapiro wrote:
              >> On Jul 26, 2010, at 2:31 PM, björn wrote:
              >>
              >>> On 12 May 2010 21:15, Michael Shapiro wrote:
              >>>> On May 12, 2010, at 1:39 PM, björn wrote:
              >>>>
              >>>>> Should this patch be made against mainline Vim instead of MacVim (i.e.
              >>>>> is there any other platform that might benefit from it)? If so, could
              >>>>> you please create a new patch and send it on to vim_dev? Otherwise
              >>>>> I'll merge it with the MacVim repo.
              >>>>
              >>>> I pulled the vim 7.2 source from http://www.vim.org/sources.php and poked around for ruby config arguments. The 'provide your own ruby' option is missing.
              >>>>
              >>>> vim72 $ ./configure --help | grep ruby
              >>>> --enable-rubyinterp Include Ruby interpreter.
              >>>>
              >>>> MacVim $ ./configure --help | grep ruby
              >>>> --enable-rubyinterp Include Ruby interpreter.
              >>>> --with-ruby-command=RUBY name of the Ruby command (default: ruby)
              >>>>
              >>>> From the OS X-specific nature of the --with-ruby-command option in configure.in, it looks like it wasn't written with other platforms in mind. If it were to be submitted to vim_dev, it should probably be generalized for linux and windows.
              >>>>
              >>>>>
              >>>>> I took a quick look at the patch but it is not entirely clear to me
              >>>>> what it does? Is it only vi_cv_path_ruby that is set earlier or is
              >>>>> something else going on?
              >>>>
              >>>> Sorry about the messy patch, I was correcting the indenting at the same time.
              >>>>
              >>>> Here's a far better patch. http://gist.github.com/398996
              >>>>
              >>>> The problems were:
              >>>> 1. The original --with-ruby-command assumed that the provided ruby was in the users PATH, and not an absolute path to a ruby binary. (AC_PATH_PROG by default looks in the user's PATH). I've corrected this by providing the path to the binary to AC_PATH_PROG.
              >>>>
              >>>> 2. If a custom ruby is provided, we don't want to use -framework Ruby. There's a conditional to check for that now.
              >>>>
              >>>> 3. The bit of ruby used as a fallback to provide library directories assumed that "." would be one of them. I've corrected that.
              >>>>
              >>>> The embedded conditional does some checks for ruby on previous versions of OS X, I don't know enough about the state of old versions, so I just left it as the default behavior if an absolute-pathed ruby isn't provided.
              >>>>
              >>>> Hope that clear things up a bit. :)
              >>>
              >>> Mike, could you do me a favor and update this patch against the latest
              >>> source code version [1] and repost it. Also, please use
              >>> git-format-patch so that you get listed as the author of the patch.
              >>> As soon as you send me an updated patch I'll take a look at it and
              >>> merge asap.
              >>>
              >>> Sorry for taking so long to respond to this patch!
              >>>
              >>> Thanks,
              >>> Björn
              >>>
              >>>
              >>> [1] http://github.com/b4winckler/macvim
              >>
              >> You got it, dude.
              >>
              >> Tested on 10.6.4 with ruby 1.9.1-p378 built by RVM.
              >
              > Thanks, but there is a problem:
              >
              > checking --enable-rubyinterp argument... yes
              > checking --with-ruby-command argument... defaulting to ruby
              > ./configure: line 5780: test: !=: unary operator expected
              > checking for ruby... /usr/bin/ruby
              > checking Ruby version... OK
              > checking Ruby header files...
              > /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/universal-darwin10.0
              >
              > Line 5780 comes from the following fragment in configure.in:
              >
              > if test $RUBY_PATH != $RUBY_CMD; then
              >
              > Can you fix this and resend the patch? Also, be careful with
              > indentation, make sure you have "set noet" (some lines were not
              > indented correctly).
              >
              > Thanks,
              > Björn


              Hm, Sorry about that.

              I've fixed the indenting and removed the need for that conditional completely.

              --Mike
            • björn
              ... Hmmm...have you re-indented lots of lines that your patch didn t change by some chance? All of a sudden the patch touches a lot more lines than the
              Message 6 of 16 , Jul 28, 2010
                On 27 July 2010 02:34, Michael Shapiro wrote:
                >
                > On Jul 26, 2010, at 5:31 PM, björn wrote:
                >>
                >> Thanks, but there is a problem:
                >>
                >> checking --enable-rubyinterp argument... yes
                >> checking --with-ruby-command argument... defaulting to ruby
                >> ./configure: line 5780: test: !=: unary operator expected
                >> checking for ruby... /usr/bin/ruby
                >> checking Ruby version... OK
                >> checking Ruby header files...
                >> /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/universal-darwin10.0
                >>
                >> Line 5780 comes from the following fragment in configure.in:
                >>
                >>  if test $RUBY_PATH != $RUBY_CMD; then
                >>
                >> Can you fix this and resend the patch?  Also, be careful with
                >> indentation, make sure you have "set noet" (some lines were not
                >> indented correctly).
                >
                > Hm, Sorry about that.
                >
                > I've fixed the indenting and removed the need for that conditional completely.

                Hmmm...have you re-indented lots of lines that your patch didn't
                change by some chance? All of a sudden the patch touches a lot more
                lines than the previous patch. Please take another look at the patch
                and send me a minimal patch, otherwise it is difficult for me to see
                what has actually changed. (Also, you only need to include
                src/configure.in -- I'll generate a new src/configure on my own when I
                test the patch.)

                Thanks,
                Björn

                --
                You received this message from the "vim_mac" maillist.
                Do not top-post! Type your reply below the text you are replying to.
                For more information, visit http://www.vim.org/maillist.php
              • björn
                ... Now that the 7.3 release is out of the way I have some time to merge this patch. Michael: can you send me a new (clean) patch based on the latest commit
                Message 7 of 16 , Aug 24, 2010
                  On 28 July 2010 22:10, björn wrote:
                  > On 27 July 2010 02:34, Michael Shapiro wrote:
                  >>
                  >> On Jul 26, 2010, at 5:31 PM, björn wrote:
                  >>>
                  >>> Thanks, but there is a problem:
                  >>>
                  >>> checking --enable-rubyinterp argument... yes
                  >>> checking --with-ruby-command argument... defaulting to ruby
                  >>> ./configure: line 5780: test: !=: unary operator expected
                  >>> checking for ruby... /usr/bin/ruby
                  >>> checking Ruby version... OK
                  >>> checking Ruby header files...
                  >>> /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/universal-darwin10.0
                  >>>
                  >>> Line 5780 comes from the following fragment in configure.in:
                  >>>
                  >>>  if test $RUBY_PATH != $RUBY_CMD; then
                  >>>
                  >>> Can you fix this and resend the patch?  Also, be careful with
                  >>> indentation, make sure you have "set noet" (some lines were not
                  >>> indented correctly).
                  >>
                  >> Hm, Sorry about that.
                  >>
                  >> I've fixed the indenting and removed the need for that conditional completely.
                  >
                  > Hmmm...have you re-indented lots of lines that your patch didn't
                  > change by some chance?  All of a sudden the patch touches a lot more
                  > lines than the previous patch.  Please take another look at the patch
                  > and send me a minimal patch, otherwise it is difficult for me to see
                  > what has actually changed.  (Also, you only need to include
                  > src/configure.in -- I'll generate a new src/configure on my own when I
                  > test the patch.)

                  Now that the 7.3 release is out of the way I have some time to merge
                  this patch. Michael: can you send me a new (clean) patch based on the
                  latest commit (note that the repo is now at [1])?

                  Thanks,
                  Björn

                  [1] http://github.com/b4winckler/macvim

                  --
                  You received this message from the "vim_mac" maillist.
                  Do not top-post! Type your reply below the text you are replying to.
                  For more information, visit http://www.vim.org/maillist.php
                Your message has been successfully submitted and would be delivered to recipients shortly.