Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add :next and :previous commands for vim keymap #1075

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add :next and :previous commands for vim keymap #1075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3c2390a
Add :next and :previous commands (ignoring difference between :next a…
quickdudley Apr 23, 2018
6199afc
Prevent space leak
quickdudley Apr 23, 2018
5fde11d
Exchange semantics for :previous and :next (account for how new buffe…
quickdudley Apr 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions yi-keymap-vim/src/Yi/Keymap/Vim/Ex.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ module Yi.Keymap.Vim.Ex

import Yi.Keymap.Vim.Common (EventString)
import qualified Yi.Keymap.Vim.Ex.Commands.Buffer as Buffer (parse)
import qualified Yi.Keymap.Vim.Ex.Commands.BufferCycle as BufferCycle (parse)
import qualified Yi.Keymap.Vim.Ex.Commands.BufferDelete as BufferDelete (parse)
import qualified Yi.Keymap.Vim.Ex.Commands.BufferNew as BufferNew (parse)
import qualified Yi.Keymap.Vim.Ex.Commands.Buffers as Buffers (parse)
Expand Down Expand Up @@ -48,6 +49,7 @@ import Yi.Keymap.Vim.Ex.Types (ExCommand (..), evStrin
defExCommandParsers :: [EventString -> Maybe ExCommand]
defExCommandParsers =
[ Buffer.parse
, BufferCycle.parse
, Buffers.parse
, BufferDelete.parse
, BufferNew.parse
Expand Down
34 changes: 34 additions & 0 deletions yi-keymap-vim/src/Yi/Keymap/Vim/Ex/Commands/BufferCycle.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{-# LANGUAGE OverloadedStrings #-}
module Yi.Keymap.Vim.Ex.Commands.BufferCycle (parse) where

import Control.Applicative (Alternative ((<|>)))
import Control.Monad
import Data.List
import qualified Data.Attoparsec.Text as P
import Lens.Micro.Platform
import Yi.Editor
import Yi.Keymap (Action (EditorA))
import Yi.Window
import Yi.Keymap.Vim.Common (EventString)
import qualified Yi.Keymap.Vim.Ex.Commands.Common as Common
import Yi.Keymap.Vim.Ex.Types (ExCommand (cmdAction, cmdShow))

parse :: EventString -> Maybe ExCommand
parse = Common.parse $ do
void (P.char 'b') <|> return ()
direction <- (fmap (const False) $ P.string "prev" <|> P.string "previous") <|> fmap (const True) (P.string "next")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P.string "prev" <|> P.string "previous" is redundant, because the "prev" parser also matches "previous". Maybe this behavior should be changed, because now "prevsomething" is also accepted.

Copy link
Member

@noughtmare noughtmare Jun 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be solved by using the endOfInput parser

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened #1085 to track this issue for all parsers.

return $ Common.pureExCommand {
cmdShow = if direction then "next" else "previous",
cmdAction = EditorA $ currentWindowA %= \window -> let
cl = bufkey window : bufAccessList window
cl' = if direction
then last cl : init cl
else tail cl ++ [head cl]
in forceSpine cl' `seq` window { bufkey = head cl', bufAccessList = tail cl' }
Copy link
Member

@noughtmare noughtmare Jun 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain how this would cause a memory leak without forceSpine? Isn't the entire list stored in memory anyway?

Edit: I think I see it, the reference to the old cl is kept.

Edit 2: Are you sure this works? I tried this:

main = do
  print $ iterate f [1..100] !! 1000000
  where
    f x = forceSpine r `seq` r
      where
        r = tail x ++ [head x]
    forceSpine = go
      where
        go [] = ()
        go (_:xs) = go xs

And that still has the leak.

Edit 3: Even this still has the leak:

import Data.List
main = do
  print $ foldl' (\xs _ -> f xs) [1..100] (replicate 1000000 ())
  where
    f x = forceSpine r `seq` r
      where
        r = tail x ++ [head x]
    forceSpine = go
      where
        go [] = ()
        go (_:xs) = go xs

Edit 4: This fixes it:

main = do
  print $ iterate f [1..100] !! 1000000
  where
    f x = forceSpine r `seq` r
      where
        r = tail x ++ [head x]
    forceSpine = go
      where
        go [] = ()
        go (x:xs) = x `seq` go xs

}

{-# INLINE forceSpine #-}
forceSpine :: [a] -> ()
forceSpine = go where
go [] = ()
go (_:r) = go r
1 change: 1 addition & 0 deletions yi-keymap-vim/yi-keymap-vim.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ library
Yi.Keymap.Vim.EventUtils
Yi.Keymap.Vim.Ex
Yi.Keymap.Vim.Ex.Commands.Buffer
Yi.Keymap.Vim.Ex.Commands.BufferCycle
Yi.Keymap.Vim.Ex.Commands.BufferDelete
Yi.Keymap.Vim.Ex.Commands.Buffers
Yi.Keymap.Vim.Ex.Commands.Cabal
Expand Down