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

Re: Patch to add a Recent Files menu to MacVim

Expand Messages
  • Nico Weber
    ... That s not mutually exclusive in my eyes. If you prefer a Vim recent files plugin, you can use it. If you want the GUI recent files menu, it s now there,
    Message 1 of 10 , May 4, 2008
    • 0 Attachment
      > Not that I'm opposed to a recent files menu, but shouldn't this be
      > done the Vim
      > way rather than a MacVim way?

      That's not mutually exclusive in my eyes. If you prefer a Vim recent
      files plugin, you can use it. If you want the GUI recent files menu,
      it's now there, too. Note that the patch also adds the files you open
      with File->Open to the global Recent Items menu (which you can access
      by clicking on the apple in the upper left corner of your screen) --
      that's not possible in a Vim-only way (well, we could a vimscript
      function addrecentfile() that does this on "systems that support a
      recent file list", a "clearrecentfiles()" function, a vimscript list
      "recentfiles" that returns the recent file list provided by the
      system, etc and then somehow implement this in vimscript. But that's
      like 5 times as much code, and I believe a Recent Files menu is only
      customary on OS X anyways).

      >

      I guess I just want to say that I won't write a vimscript version of
      this patch. If someone else provides a vimscript solution that works
      just as well (where "just as well" means "nearly exactly like the
      usual Recent Files menu on OS X"), I wouldn't mind if that patch was
      included instead :-)

      Nico


      --~--~---------~--~----~------------~-------~--~----~
      You received this message from the "vim_mac" maillist.
      For more information, visit http://www.vim.org/maillist.php
      -~----------~----~----~----~------~----~------~--~---
    • björn
      Nico, I have looked over the patch a bit and made some minor changes (attached). I m a bit confused about the memory handling. As you ll see in the patch I m
      Message 2 of 10 , May 9, 2008
      • 0 Attachment
        Nico,

        I have looked over the patch a bit and made some minor changes
        (attached). I'm a bit confused about the memory handling. As you'll
        see in the patch I'm releasing the "recentFilesMenu" since this was a
        memory leak. However, I'm not sure if you were assuming it was not
        released when you call replace:with: (I also changed the name of that
        message for now...it probably should be moved as well.)? I'm not sure
        something might go wrong when you do the swapping of menu items since
        one item is released (the one that is swapped out)...it seems a bit
        precarious to me. Have you thought about it?

        Also, it makes more sense to me to add files to the recent files menu
        in application:openFiles: instead of in fileOpen:. This way, files
        that are double-clicked, dragged to the dock icon, etc. will also be
        put on the recent files menu (and not only files that are opened via
        Cmd-o). However, I make sure files that are being edited remotely
        aren't added to the recent files menu.

        Finally, I am trying to come up with a better way to deal with menus
        so that we don't have to do these kind of hacks just to get the recent
        files menu added, so somewhere down the track we will hopefully be
        able to clean it up a bit.


        Björn

        --~--~---------~--~----~------------~-------~--~----~
        You received this message from the "vim_mac" maillist.
        For more information, visit http://www.vim.org/maillist.php
        -~----------~----~----~----~------~----~------~--~---
      • Nico Weber
        Hi, ... Looks good to me. ... Feel free to move it anywhere you want :-P ... In the first call to replaceMenuItem:, the item that s swapped out is
        Message 3 of 10 , May 9, 2008
        • 0 Attachment
          Hi,

          > I have looked over the patch a bit and made some minor changes
          > (attached).

          Looks good to me.

          > I'm a bit confused about the memory handling. As you'll
          > see in the patch I'm releasing the "recentFilesMenu" since this was a
          > memory leak. However, I'm not sure if you were assuming it was not
          > released when you call replace:with: (I also changed the name of that
          > message for now...it probably should be moved as well.)?

          Feel free to move it anywhere you want :-P

          > I'm not sure
          > something might go wrong when you do the swapping of menu items since
          > one item is released (the one that is swapped out)...it seems a bit
          > precarious to me. Have you thought about it?

          In the first call to replaceMenuItem:, the item that's swapped out is
          recentFilesMenuItem, which is retained by all VimControllers (they all
          share just one recentFilesMenuItem, doesn't work otherwise) and by the
          AppController, so this is ok. In the second call, the first argument
          it the recentFilesDummy, which should be retained recentFilesMenuItem,
          because [recentFilesMenuItem setRepresentedObject:] is called before.

          And when setRepresentedObject: is called later again and another
          representedObject is set, the old representedObject is swapped in.

          So, if a MenuItem retains its representedObject, this should work.

          > Also, it makes more sense to me to add files to the recent files menu
          > in application:openFiles: instead of in fileOpen:. This way, files
          > that are double-clicked, dragged to the dock icon, etc. will also be
          > put on the recent files menu (and not only files that are opened via
          > Cmd-o). However, I make sure files that are being edited remotely
          > aren't added to the recent files menu.

          Ok.

          > Finally, I am trying to come up with a better way to deal with menus
          > so that we don't have to do these kind of hacks just to get the recent
          > files menu added, so somewhere down the track we will hopefully be
          > able to clean it up a bit.

          Sounds good :-)

          Nico

          --~--~---------~--~----~------------~-------~--~----~
          You received this message from the "vim_mac" maillist.
          For more information, visit http://www.vim.org/maillist.php
          -~----------~----~----~----~------~----~------~--~---
        • björn
          ... Ok, but shouldn t the recentFilesDummy still be retained in addMenuItemWithTag and then released in dealloc? Or is there a reason why you don t retain it?
          Message 4 of 10 , May 9, 2008
          • 0 Attachment
            2008/5/9 Nico Weber <nicolasweber@...>:
            >
            >> I'm not sure
            >> something might go wrong when you do the swapping of menu items since
            >> one item is released (the one that is swapped out)...it seems a bit
            >> precarious to me. Have you thought about it?
            >
            > In the first call to replaceMenuItem:, the item that's swapped out is
            > recentFilesMenuItem, which is retained by all VimControllers (they all
            > share just one recentFilesMenuItem, doesn't work otherwise) and by the
            > AppController, so this is ok. In the second call, the first argument
            > it the recentFilesDummy, which should be retained recentFilesMenuItem,
            > because [recentFilesMenuItem setRepresentedObject:] is called before.
            >
            > And when setRepresentedObject: is called later again and another
            > representedObject is set, the old representedObject is swapped in.
            >
            > So, if a MenuItem retains its representedObject, this should work.

            Ok, but shouldn't the recentFilesDummy still be retained in
            addMenuItemWithTag and then released in dealloc? Or is there a reason
            why you don't retain it?

            Björn

            --~--~---------~--~----~------------~-------~--~----~
            You received this message from the "vim_mac" maillist.
            For more information, visit http://www.vim.org/maillist.php
            -~----------~----~----~----~------~----~------~--~---
          • Nico Weber
            ... Retaining it once more doesn t hurt, and it s probably clearer. ... No…I just figured that some other object always retains the dummy item, so this is
            Message 5 of 10 , May 9, 2008
            • 0 Attachment
              > Ok, but shouldn't the recentFilesDummy still be retained in
              > addMenuItemWithTag and then released in dealloc?

              Retaining it once more doesn't hurt, and it's probably clearer.

              > Or is there a reason why you don't retain it?

              No…I just figured that some other object always retains the dummy
              item, so this is not necessary. But retaining it one more time is more
              explicit and less brittle, so that's fine with me too ;-)

              Thanks,
              Nico


              --~--~---------~--~----~------------~-------~--~----~
              You received this message from the "vim_mac" maillist.
              For more information, visit http://www.vim.org/maillist.php
              -~----------~----~----~----~------~----~------~--~---
            • Nico Weber
              ... Thanks for merging! :-) Perhaps the Open Recent should be moved two items down though, so that it s directly above the separator. This is were Open
              Message 6 of 10 , May 9, 2008
              • 0 Attachment
                >> Ok, but shouldn't the recentFilesDummy still be retained in
                >> addMenuItemWithTag and then released in dealloc?
                >
                > Retaining it once more doesn't hurt, and it's probably clearer.
                >
                >> Or is there a reason why you don't retain it?
                >
                > No…I just figured that some other object always retains the dummy
                > item, so this is not necessary. But retaining it one more time is more
                > explicit and less brittle, so that's fine with me too ;-)

                Thanks for merging! :-)

                Perhaps the "Open Recent" should be moved two items down though, so
                that it's directly above the separator. This is were Open Recent is in
                all other apps I just checked (the other apps don't have quite as many
                "open" menu items as MacVim though). It looks nicer, too.

                Patch (likely overkill :-P):

                diff --git a/src/MacVim/gvimrc b/src/MacVim/gvimrc
                index ca9910c..d0db066 100644
                --- a/src/MacVim/gvimrc
                +++ b/src/MacVim/gvimrc
                @@ -45,7 +45,7 @@ aunmenu File.Save-Exit
                an <silent> 10.290 File.New\ Window :maca newWindow:<CR>
                an 10.295 File.New\ Tab :tabnew<CR>
                an <silent> 10.310 File.&Open\.\.\. :maca fileOpen:<CR>
                -an <silent> 10.314 File.Open\ Recent :maca
                recentFilesDummy:<CR>
                +an <silent> 10.327 File.Open\ Recent :maca
                recentFilesDummy:<CR>
                an 10.328 File.-SEP0- <Nop>
                an <silent> 10.330 File.Close\ Window<Tab>:qa :confirm qa<CR>
                an <silent> 10.331 File.Close :maca
                performClose:<CR>

                Thanks,
                Nico
                --~--~---------~--~----~------------~-------~--~----~
                You received this message from the "vim_mac" maillist.
                For more information, visit http://www.vim.org/maillist.php
                -~----------~----~----~----~------~----~------~--~---
              • björn
                ... Ok. I ve moved it downwards now. I also pushed a patch which ensures that files that are opened/saved from a :browse command are added to the Recent
                Message 7 of 10 , May 10, 2008
                • 0 Attachment
                  2008/5/9 Nico Weber <nicolasweber@...>:
                  >
                  > Perhaps the "Open Recent" should be moved two items down though, so
                  > that it's directly above the separator. This is were Open Recent is in
                  > all other apps I just checked (the other apps don't have quite as many
                  > "open" menu items as MacVim though). It looks nicer, too.

                  Ok. I've moved it downwards now. I also pushed a patch which ensures
                  that files that are opened/saved from a :browse command are added to
                  the "Recent Files" menu. It got kind of confusing otherwise since
                  "Open..." would put a file on the "Recent Files" menu but
                  "Split-Open..." and "Open Tab..." wouldn't. Well, now they do.

                  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.