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

Patch to add a Recent Files menu to MacVim

Expand Messages
  • Nico Weber
    Hi, the attached patch adds a Recent Files menu to MacVim. For now, only files opened via File- Open show up. Files are added to the Recent Files menu by
    Message 1 of 10 , May 3, 2008
    • 0 Attachment
      Hi,

      the attached patch adds a Recent Files menu to MacVim. For now, only
      files opened via File->Open show up.

      Files are added to the Recent Files menu by calling
      noteNewRecentDocumentURL: on NSDocumentController. I'm currently only
      calling this in -[MMAppController fileOpen:]. Perhaps it should be
      called in other places too?

      The Recent Files is added at the position of a menu item with the
      action recentFilesDummy:. This action doesn't do anything, it only
      marks the spot where the Recent Files menu should be put.

      Nico


      --~--~---------~--~----~------------~-------~--~----~
      You received this message from the "vim_mac" maillist.
      For more information, visit http://www.vim.org/maillist.php
      -~----------~----~----~----~------~----~------~--~---
    • Ben Schmidt
      ... Shouldn t this integrate with some (existing or new) recent files Vim plugin, and just provide GUI access to the same datastore that plugin uses? Or for
      Message 2 of 10 , May 4, 2008
      • 0 Attachment
        Nico Weber wrote:
        > Hi,
        >
        > the attached patch adds a Recent Files menu to MacVim. For now, only
        > files opened via File->Open show up.

        Shouldn't this integrate with some (existing or new) recent files Vim plugin, and
        just provide GUI access to the same datastore that plugin uses? Or for that
        matter, since such plugins usually add the recent files to a menu anyway, IIRC,
        actually need no MacVim specific implementation at all?

        Not that I'm opposed to a recent files menu, but shouldn't this be done the Vim
        way rather than a MacVim way?

        Ben.

        > Files are added to the Recent Files menu by calling
        > noteNewRecentDocumentURL: on NSDocumentController. I'm currently only
        > calling this in -[MMAppController fileOpen:]. Perhaps it should be
        > called in other places too?
        >
        > The Recent Files is added at the position of a menu item with the
        > action recentFilesDummy:. This action doesn't do anything, it only
        > marks the spot where the Recent Files menu should be put.





        --~--~---------~--~----~------------~-------~--~----~
        You received this message from the "vim_mac" maillist.
        For more information, visit http://www.vim.org/maillist.php
        -~----------~----~----~----~------~----~------~--~---
      • björn
        ... The problem with your suggestion is that MacVim needs to coordinate the recent files from each Vim process, which may be a bit tricky. (Each Vim process
        Message 3 of 10 , May 4, 2008
        • 0 Attachment
          2008/5/4 Ben Schmidt <mail_ben_schmidt@...>:
          >
          > Nico Weber wrote:
          > > Hi,
          > >
          > > the attached patch adds a Recent Files menu to MacVim. For now, only
          > > files opened via File->Open show up.
          >
          > Shouldn't this integrate with some (existing or new) recent files Vim plugin, and
          > just provide GUI access to the same datastore that plugin uses? Or for that
          > matter, since such plugins usually add the recent files to a menu anyway, IIRC,
          > actually need no MacVim specific implementation at all?
          >
          > Not that I'm opposed to a recent files menu, but shouldn't this be done the Vim
          > way rather than a MacVim way?

          The problem with your suggestion is that MacVim needs to coordinate
          the "recent files" from each Vim process, which may be a bit tricky.
          (Each Vim process has its own menu so it would overwrite any "recent
          files" menu of another process.) For this reason and since "recent
          files" isn't a standard item in the other GUI ports anyway (but it
          _is_ a standard for OS X apps), I don't think Nico's approach is all
          that bad.

          Nico, thanks for the patch. I haven't had a chance to look at it yet,
          but I'll let you know when I have.

          /Björn

          --~--~---------~--~----~------------~-------~--~----~
          You received this message from the "vim_mac" maillist.
          For more information, visit http://www.vim.org/maillist.php
          -~----------~----~----~----~------~----~------~--~---
        • 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 4 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 5 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 6 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 7 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 8 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 9 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 10 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.