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

View patches and git-fu

Expand Messages
  • Nico Weber
    Hi all, perhaps I ve sent out a patch that completes most of the vimview refactoring (I m still getting used to git). Perhaps I ve sent this patch as a series
    Message 1 of 6 , Oct 30, 2007
      Hi all,

      perhaps I've sent out a patch that completes most of the vimview
      refactoring (I'm still getting used to git). Perhaps I've sent this
      patch as a series of 15 patches. Sorry about that.

      Anyways, if everything works just as it did before I'd be happy :-)
      This fixes the `:set lines=1000000` problem in fu mode (but not for
      columns -- setting columns to something huge in non-fu mode doesn't
      work either, so... :-P), and perhaps Cmd-o works in fu mode on ppc
      system as well now. Furthermore, this fixes a regression that the tab
      sizes weren't adjusted when changing the window size.

      Fixes to the other problems should hopefully be faster now :-\

      Bye,
      Nico

      --~--~---------~--~----~------------~-------~--~----~
      You received this message from the "vim_mac" maillist.
      For more information, visit http://www.vim.org/maillist.php
      -~----------~----~----~----~------~----~------~--~---
    • Nico Weber
      ... ...doesn t look like it. The patches are attached (how can I get a single patch? I used `git pull` to update my repo, then did `git rebase` to merge my
      Message 2 of 6 , Oct 30, 2007
        > perhaps I've sent out a patch that completes most of the vimview
        > refactoring (I'm still getting used to git). Perhaps I've sent this
        > patch as a series of 15 patches. Sorry about that.

        ...doesn't look like it. The patches are attached (how can I get a
        single patch? I used `git pull` to update my repo, then did `git
        rebase` to merge my changes to the master branch. Is there a better
        way?).

        Nico

        ps: `git diff --stat`is nice :-)


        --~--~---------~--~----~------------~-------~--~----~
        You received this message from the "vim_mac" maillist.
        For more information, visit http://www.vim.org/maillist.php
        -~----------~----~----~----~------~----~------~--~---
      • Tim Allen
        ... git-send-email requires that the patches produced by git-format-patch be named on the command-line. Something like git-send-email --blah *.patch should
        Message 3 of 6 , Oct 30, 2007
          On Oct 30, 10:01 pm, Nico Weber <nicolaswe...@...> wrote:
          > > perhaps I've sent out a patch that completes most of the vimview
          > > refactoring (I'm still getting used to git). Perhaps I've sent this
          > > patch as a series of 15 patches. Sorry about that.
          >
          > ...doesn't look like it.

          git-send-email requires that the patches produced by git-format-patch
          be named on the command-line. Something like "git-send-email --blah
          *.patch" should do the trick.

          > The patches are attached (how can I get a
          > single patch? I used `git pull` to update my repo, then did `git
          > rebase` to merge my changes to the master branch. Is there a better
          > way?).

          The Git Way is to send out a lot of smaller patches that each do one
          single thing, rather than a huge monolithic patch that can't be
          effectively reviewed. Here's a good example of a recent thread on the
          git-mailing list that does this:

          http://thread.gmane.org/gmane.comp.version-control.git/62498

          If you *did* want to produce a single big diff, the simplest way is
          just to look at the commit IDs of the first and last commits you want
          to include (git-log is good for this), then use git-diff. For example,
          if the first commit ID was something like "abcdef", and the last
          commit ID was something like "012345", then you could just write:

          git-diff --binary abcdef..012345 > ~/my-changes.patch

          (the '--binary' option makes sure the patch includes any changes made
          to binary files, instead of just saying "a binary file was changed")

          The more sophisticated way to produce a patch-series from your hacking
          is to create a branch off the current upstream trunk, then use git-
          cherry-pick to pull changes from your working branch one by one, in a
          sensible, logical order. With careful use of the --no-commit option,
          you can even hide all your "oops" commits so that it looks like you
          got everything working perfectly first time. :)

          If you're really, REALLY sure you know how you want to restructure
          your patch series before you begin, you can use git-rebase --
          interactive, tell it what to do, then sit back and watch the patches
          fly:

          http://blog.madism.org/index.php/2007/09/09/138-git-awsome-ness-git-rebase-interactive


          --~--~---------~--~----~------------~-------~--~----~
          You received this message from the "vim_mac" maillist.
          For more information, visit http://www.vim.org/maillist.php
          -~----------~----~----~----~------~----~------~--~---
        • Tim Allen
          ... Confirmed. ... Confirmed. :) That is to say, setting columns to a large number gives you a very wide window that expands off your screen to the left and
          Message 4 of 6 , Oct 30, 2007
            On Oct 30, 9:57 pm, Nico Weber <nicolaswe...@...> wrote:
            > Anyways, if everything works just as it did before I'd be happy :-)
            > This fixes the `:set lines=1000000` problem in fu mode

            Confirmed.

            > (but not for
            > columns -- setting columns to something huge in non-fu mode doesn't
            > work either, so... :-P),

            Confirmed. :)

            That is to say, setting 'columns' to a large number gives you a very
            wide window that expands off your screen to the left and right.

            > and perhaps Cmd-o works in fu mode on ppc
            > system as well now.

            It seems not: when in fullscreen mode, <D-o> sets 'lines' and
            'columns' to the full size of the screen, drops down a file-picker
            sheet, and then (once a file is chosen and the user clicks OK),
            nothing happens. If I hit <D-o> again and choose a different file and
            click OK, then the second file shows up. (if I try again but pick the
            files in the opposite order, it's still the second one that shows up,
            so it's not a problem with one of the files).

            If I could guess from the commit-summaries which of your patches was
            supposed to fix this bug, I might investigate to see if I could shed
            any light on the problem, but I can't guess and it's past midnight, so
            I'll go to bed. :)


            --~--~---------~--~----~------------~-------~--~----~
            You received this message from the "vim_mac" maillist.
            For more information, visit http://www.vim.org/maillist.php
            -~----------~----~----~----~------~----~------~--~---
          • björn
            ... I have pushed this to the public repo. Two changes to your patches: 1. MMWindowController.m line 657: pass content rect size instead of window rect size
            Message 5 of 6 , Oct 30, 2007
              On 30/10/2007, Nico Weber <nicolasweber@...> wrote:
              > > perhaps I've sent out a patch that completes most of the vimview
              > > refactoring (I'm still getting used to git). Perhaps I've sent this
              > > patch as a series of 15 patches. Sorry about that.

              I have pushed this to the public repo. Two changes to your patches:
              1. MMWindowController.m line 657: pass content rect size instead of
              window rect size to resizeVimViewToFitSize (this caused problem if you
              did ":set lines=1000" twice)
              2. Deregister vim view as notification observer in [MMVimView cleanup].

              I'll share some wisdom regarding item 2. Failing to deregister as a
              notification observer can be fatal. Nothing bad will happen straight
              away, but after a while the app may start behaving VERY strangely. I
              spent something like two days tracking a bug once only to find out
              that the problem was PSMTabBarControl not deregistering as a
              notification observer! :-(


              > ...doesn't look like it. The patches are attached (how can I get a
              > single patch? I used `git pull` to update my repo, then did `git
              > rebase` to merge my changes to the master branch. Is there a better
              > way?).

              I don't know. What I did is I took your patches, applied them to a
              new branch then merged this branch with my master using "git-merge
              --squash" to make it look like one commit. Then I added the commit
              message and set you as the author of the patch with "git-commit
              --author "Nico...". It would be good if I could just apply the patch
              once and get a nice commit message that you've written...but I don't
              know how to do this myself. :-)

              Finally, some observations regarding this patch:
              - :set lines=1000 then :set lines+=1 will first maximize the view, but
              then the second command will make it slightly larger. The second
              command should be a no-op. Actually, why do you use the window size
              instead of the screen size when computing how large the view can be?
              This way it is impossible to make the view really fill the screen in
              'fu'.

              - Some methods has "actualRows" as part of their names. This is
              misleading at best since "actualRows" is a private variable that
              should only be known to the text storage (and the typesetter). Just
              take out the "actual" part from the method names and it will be good.

              - Do we still need a "placeViews" method in MMWindowController? Maybe
              it should be called something else now?

              Anyway, it is starting to look much better. Thanks for taking on this
              task (the code you are refactoring is some of the most difficult in
              all of MacVim!).


              /Björn

              --~--~---------~--~----~------------~-------~--~----~
              You received this message from the "vim_mac" maillist.
              For more information, visit http://www.vim.org/maillist.php
              -~----------~----~----~----~------~----~------~--~---
            • björn
              ... I forgot to mention that this bug only happens in full-screen mode, by the way. /Björn --~--~---------~--~----~------------~-------~--~----~ You received
              Message 6 of 6 , Oct 30, 2007
                On 30/10/2007, björn <bjorn.winckler@...> wrote:
                > - :set lines=1000 then :set lines+=1 will first maximize the view, but
                > then the second command will make it slightly larger. The second
                > command should be a no-op. Actually, why do you use the window size
                > instead of the screen size when computing how large the view can be?
                > This way it is impossible to make the view really fill the screen in
                > 'fu'.

                I forgot to mention that this bug only happens in full-screen mode, by the way.

                /Björn

                --~--~---------~--~----~------------~-------~--~----~
                You received this message from the "vim_mac" maillist.
                For more information, visit http://www.vim.org/maillist.php
                -~----------~----~----~----~------~----~------~--~---
              Your message has been successfully submitted and would be delivered to recipients shortly.