Unclear semantics for Prompt completions

[solomon-b]

Problem Description

I'm pretty motivated to put in some work on XMonad.Prompt. There is a lot of potential improvements around navigation and scrolling through commands. However, the semantics of the completion system are very unclear and possibly buggy. Is there anyone here who can give me clear explanation of the expected behavior of the completions system?

I'm going to give some examples of weird behavior of completions. I'm hoping we can determine which of these behaviors are intended, which are bugs, and finally define the semantics for completions. If we can do that then I would like to refactor this module a bit in anticipation of doing some feature work to extend the Prompt interface.

Simple Completions (alwaysHighlight == False)

I am using an XMonad prompt because they are very simple and use the default definitions of XPrompt.

Given the prompt:

systemCtlPrompt :: X ()
systemCtlPrompt =
  let commands =
        [ ("foo", pure ())
        , ("bar", pure ())
        , ("baz", pure ())
        , ("qux", pure ())
        ]
  in xmonadPromptCT "TEST!!" commands promptConfig

Tab completion works as I would expect. If I have entered nothing into the command line then pressing tab cycles between all the completion list options. If I enter b and press tab, it cycles between bar and baz.

Now if I change the prompt to use two words per completion:

systemCtlPrompt :: X ()
systemCtlPrompt =
  let commands =
        [ ("1 foo", pure ())
        , ("2 bar", pure ())
        , ("3 baz", pure ())
        , ("4 qux", pure ())
        ]
  in xmonadPromptCT "TEST!!" commands promptConfig

Now I get what I would consider a bug. The first tab sets the prompt to 1 foo and subsequent tabs prepend the command line with more 1 s.

This behavior is actually no surprise considering the default implementation of nextCompletion:

-- | Given the prompt type, the command line and the completion list,
-- return the next completion in the list for the last word of the
-- command line. This is the default 'nextCompletion' implementation.
getNextOfLastWord :: XPrompt t => t -> String -> [String] -> String
getNextOfLastWord t c l = skipLastWord c ++ completionToCommand t (l !! ni)
    where ni = case commandToComplete t c `elemIndex` map (completionToCommand t) l of
                 Just i -> if i >= length l - 1 then 0 else i + 1
                 Nothing -> 0

Given the default XPrompt definitions, what happens here is that we lookup the last word of the command line in the completion list--that word would be foo in our case--which will never be found thus setting nl = 0. Then the command line minus the last word is prepended to the entire completion string at index 0 and returned.

Based on the haddock description of this function, I believe what it should be doing is filtering the completion list for entries which contain the last word of the command line. I have no idea why this is a useful behavior for tab completion but that is what the haddock sort of describes.

highlight completions (alwaysHighlight == True)

The hlComplete code is a lot more confusing. Not so much to follow the subroutines, but to understand the desired behavior.
The user experience of of high completions feels pretty similar to simple completions modulo edge cases.

Using the same prompt from earlier:

systemCtlPrompt :: X ()
systemCtlPrompt =
  let commands =
        [ ("1 foo", pure ())
        , ("2 bar", pure ())
        , ("3 baz", pure ())
        , ("4 qux", pure ())
        ]
  in xmonadPromptCT "TEST!!" commands promptConfig

The tab key with no command input cycles through all the available completions. If I type b then tab cycles between 2 bar and 3 baz.

It gets weird if you enter a multiword command. For example, if you type "foo " (that is with a space after foo) then you get the entire completion list. I would expect this to either yield no completions (there is no completion with "foo " as a substring) or it would yield the 1 foo completion (by stripping the trailng space from the "search").

If after typing "foo "you then press tab, the highlight stays on 1 foo, the command gets replaced with foo 1 foo, and the completion list is still unfiltered. Subsequent pressed of tab cycle the highlighted completion and change the command to that completion prepended with "foo ", until you cycle back to the first completion and the prepended foo goes away.

If we use another prompt example I can show another strange behavior:

scrotPrompt :: X ()
scrotPrompt = xmonadPromptCT "Screenshot Options" commands promptConfig
  where
    commands = [ ("1: Capture Screen", spawn "scrot")
               , ("2: Capture Selection", spawn "scrot -s")
               , ("3: Capture All Screens", spawn "scrot -m")
               , ("4: Capture with 3 second countdown", spawn "scrot -d 3 -c")
               ]

If I enter 1 as my command then the search options are filtered down to 1: Capture Screen. Now if I press tab then the command is replaced with 1: Capture Screen as expected, however the completion options are also now extended to include 3: Capture All Screens. i have not been able to replicate this behavior with any other prompts or command inputs.

Proposal

If someone can outline the intended behavior for both completion types then I would be happy to write a PR eliminating whatever edge cases are not intended as part of that behavior.

Alternatively I would propose we eliminate the distinction between both completion types and replace them with a simple fuzzy filter of the completion options based on the currently entered command and the tab key can cycle through the filtered list of completions. alwaysHighlight would be either removed or not do anything (depending on how we handle deprecation of type parameters).

LIke I said, I am motivated to work on and expand the functionality of XMonad.Prompts. I want to do some cooler stuff like pagination and multiselect but before I can get there I think we need to have sensible behavior for completions.

Checklist

• I've read CONTRIBUTING.md

• I tested my configuration

  • With xmonad commit 33a86c0cdb9aa481e23cc5527a997adef5e32d42
  • With xmonad-contrib commit 20fdcba

[slotThe]

However, the semantics of the completion system are very unclear and possibly buggy. Is there anyone here who can give me clear explanation of the expected behavior of the completions system?

[…]

I'm going to give some examples of weird behavior of completions. I'm hoping we can determine which of these behaviors are intended, which are bugs, and finally define the semantics for completions.

I don't think that anyone who extensively worked on the prompt is still active, so at this point it's patchworks all the way down :) I would say that the "expected" semantics are those who users—right now—expect to have happen. Essentially, go wild.

Many of your examples of the prompt's inconsistency essentially stem from the problem that we don't exactly know which part of the currently entered prompt the user wants to have completed. There are some examples (like the shell prompt) where a user may have entered a command, a space, and then wants a directory auto-completed without touching the initially entered command. Other times things separated by spaces are to be treated as one thing; this creates some friction.

Alternatively I would propose we eliminate the distinction between both completion types and replace them with a simple fuzzy filter of the completion options based on the currently entered command and the tab key can cycle through the filtered list of completions. alwaysHighlight would be either removed or not do anything (depending on how we handle deprecation of type parameters).

It would certainly make the code a lot simpler. I personally don't have anything against this approach (then again, I always use alwaysHighlight, so I'm not sure how people who do not use this option at the moment would react).

[solomon-b]

Many of your examples of the prompt's inconsistency essentially stem from the problem that we don't exactly know which part of the currently entered prompt the user wants to have completed. There are some examples (like the shell prompt) where a user may have entered a command, a space, and then wants a directory auto-completed without touching the initially entered command. Other times things separated by spaces are to be treated as one thing; this creates some friction.

I haven't checked all the prompts, but in the case of the shell prompt it is using the default completion implementations for XPrompt. Rather then trying to have a single universal completion function, I think we should have a very simple default implementation and then defer specialized behavior to the various XPrompt instances.

As an initial project I could rewrite the default completion code as a I described above and save the current implementation outside of the typeclass. Then we can use the old completion code in the instance definitions for all prompts that currently rely on it.

[slotThe]

On Sun, Oct 03 2021 16:35, Solomon Bothwell wrote: As an initial project I could rewrite the default completion code as a I described above and save the current implementation outside of the typeclass. Then we can use the old completion code in the instance definitions for all prompts that currently rely on it.

Go wild 👍

[solomon-b]

Go wild +1

Will do :)

[solomon-b]

I'm still working on this. I'm putting together a more minimal prompt design in a seperate cabal project for more rapid prototyping. It isn't ready to discuss here but there is another Prompt related issue I would like to bring up.

The XPrompt typeclass is designed and used in a really odd way. It looks like an attempt at OOP in haskell that at the end of the day is using typeclass dictionaries to emulate the behavior of a plain record of functions. ie., we define a bunch of functions in a typeclass instance and then use a data constructor (XPType) to existentialize the instance and load it into our state.

I think we should just turn XPrompt into a record and call it a day. Thoughts?

[slotThe]

I think we should just turn XPrompt into a record and call it a day. Thoughts?

Would this be as extensible as the current system? It seems less so, but this may just be my initial impression.

It seems like we would just switch from having an "up-front cost" by using type classes to delaying this until later. E.g., especially when using multiple modes it's currently very easy to decide which function to call exactly (just query currentXPMode and be on your way). This would seem to be a bit more work when dropping existentials and making this a normal record of functions.

Another thing to think about here is just how much user code this would break

[solomon-b]

Would this be as extensible as the current system? It seems less so, but this may just be my initial impression.

Unless I am missing something subtle, we would have identical behavior but the code would be a lot more explicit.

It seems like we would just switch from having an "up-front cost" by using type classes to delaying this until later. E.g., especially when using multiple modes it's currently very easy to decide which function to call exactly (just query currentXPMode and be on your way). This would seem to be a bit more work when dropping existentials and making this a normal record of functions.

currentXPMode would work exactly the same way. The XPrompt type would be a record contaning all the functions currently in theXPrompt class (or whatever we decided to change them to when we clean up the semantics). Then we throw away XPType. OperationMode would then contain XPrompt values directly:

data XPrompt = XPrompt { showPrompt :: String, nextCompletion :: String -> [String] -> String, ... }

data XPOperationMode = XPSingleMode ComplFunction XPrompt | XPMultipleModes (W.Stack XPrompt)

currentXPMode :: XPState -> XPrompt
currentXPMode st = case operationMode st of
  XPMultipleModes modes -> W.focus modes
  XPSingleMode _ xprompt -> xprompt

Another thing to think about here is just how much user code this would break

How many people are actually writing their own XPrompt instances in their personal configs? I am under the impression that it is primarily used for writing other Prompt modules in xmonad-contrib.

[slotThe]

Unless I am missing something subtle, we would have identical behavior but the code would be a lot more explicit.

Mh interesting. I would almost say "go for it" if we didn't have to at least somewhat keep backwards compatibility in mind :/

How many people are actually writing their own XPrompt instances in their personal configs? I am under the impression that it is primarily used for writing other Prompt modules in xmonad-contrib.

I have never understood this either, but I've seen small prompts in quite a few user configurations. This may or may not still be something that we want to do at some point, but perhaps we should take this one step at a time (first fix the completion system, then talk about refactoring the core types)

[solomon-b]

Sorry for the slow response time. I've been caught up with work stuff. We can keep the typeclass for now, but we are going to need to change it substantially. It just doesn't really capture what it means to be a prompt.

I have a working prototype in a scratch project that uses this interface:

data Prompt = Prompt
  { filterCompletions :: String -> Completions -> Maybe Completions
  -- ^ Filter the set of possible completions.
  , fetch :: [String] -> IO (Maybe [String])
  -- ^ Update the list of completions based on the current command.
  , complete :: String -> String -> String
  -- ^ Modify the last word of the command based on the focused completion.
  , execute :: String -> IO ()
  -- ^ Perform an action based on the command.
  , divider :: Maybe Char
  -- ^ Optional character for subdividing the command string
  }

Ignore the fact that I used a record, this would be the core of a new XPrompt typeclass. This design differs from the current prompt behavior but it can handle both simple prompts and the shell prompt. The goal is to capture the basic actions that distinguish prompts in this record (or typeclass).

To make it more consistent, I broke up the act of completing the Command from shifting the focus. How we complete a command is highly dependent on the particulars of the Prompt--such as with the shell prompt--but shifting focus in the completions is always the same. Internally in my demo project I used a Zipper for storing completions so we it is easy to shift focus.

I don't think the divider field should exist. That should be baked into the fetch command but I haven't tried doing that yet.

This set of functions is able to handle simple prompts as well as complex IO heavy prompts like the shell prompt. Here are two examples to show simple and IO based prompts:

-- | Prompt to select a completion from a list and print it to stdout
simplePrompt xs = Prompt
  { filterCompletions = \cmd cmpls -> filterZ (isPrefixOf cmd) cmpls
  , fetch = \case
      [] -> pure $ Just xs
      _ -> pure Nothing
  , complete = (<>)
  , execute = \s -> putStrLn $ "WOW GOOD JOB: " <> s
  , divider = Nothing
  }

lookupDirectory :: [String] -> IO (Maybe [String])
lookupDirectory cmd = do
  let path = '.' : '/' : foldl (</>) "" cmd
  exists <- doesPathExist path
  if exists
     then fmap Just $ listDirectory path
     else pure Nothing

-- | Prompt to navigate the file system using completions and build up a path to print to stdout
pathPrompt :: Prompt
pathPrompt = Prompt
  { filterCompletions = \cmd cmpls -> filterZ (isPrefixOf cmd) cmpls
  , fetch = \case
      [] -> lookupDirectory []
      cmd -> lookupDirectory cmd
  , complete = (<>)
  , execute = \s -> putStrLn $ "WOW GOOD JOB: " <> s
  , divider = Just '/'
  }

My demo project is in a larger scratch repo. I'll break it into its own repo and link it here this week so that you can have a look at it.

I'm not married to this specific design, but I think we shouldn't be stuck with 14 year old technical debt out of fear of making breaking changes for users.

[slotThe]

We can keep the typeclass for now, but we are going to need to change it substantially. It just doesn't really capture what it means to be a prompt.

[…]

Ignore the fact that I used a record, this would be the core of a new XPrompt typeclass.

If we're making a breaking change anyways then there's no harm with going all the way and using the better design. I was just under the impression that the completion refactor you were planning was going to be backwards compatible with the current interface, which is why I had reservations about using a record.

I'm not married to this specific design, but I think we shouldn't be stuck with 14 year old technical debt out of fear of making breaking changes for users.

Well, XMonad has a track record of being really rather stable and not breaking too many things (even with 0.17.0 we somehow managed to not really have any gigantic breaking changes, which this would definitely be). This certainly needs to be deliberated thoroughly.

My demo project is in a larger scratch repo. I'll break it into its own repo and link it here this week so that you can have a look at it.

That sounds good 👍 The examples looks nice and make me hopeful that we can move forward with this idea in spite of the huge breaking change it would introduce