zsh-workers
 help / color / mirror / code / Atom feed
* Re: PATCH and Re: simulation of dabbrev-expand
@ 1999-10-11 11:12 Sven Wischnowsky
  1999-10-11 21:44 ` Adam Spiers
  0 siblings, 1 reply; 6+ messages in thread
From: Sven Wischnowsky @ 1999-10-11 11:12 UTC (permalink / raw)
  To: zsh-workers


[ Trying to reduce the number of replies by answering multiple
messages... ]

Adam Spiers wrote:

> > Although... we could do it whenever someone 
> > looks at `compstate[*nmatches]', set a flag if the list is sorted and
> > clear the flag when another match is added. Hm. No time now...
> 
> Sounds reasonable, but still feels (IMO) a bit of a workaround forced by the
> wrong design decision.  Maybe there's no good solution.  *shrug*

First, the `design decision' comes from a time when one couldn't
execute shell code (and look at the current state) during
completion. Then, I think it is still right to execute the probably
expansive sorting and uniquifying as seldom as possible -- and in most 
cases this is really only needed at the end. But I after thinking some 
more about this, I really think, the solution to do the calculation
when one of the `nmatches' keys is used is ok. Normally we do this at
the very beginning (so there are no matches and this doesn't take any
time) and at the end (and then we still do the work only once because
the stuff calculated when `compstate[nmatches]' was used at the end).

This could also be combined with...

Benjamin Korvemaker wrote:

> 2) When there are too many completions, and a prompt like
> 
> zsh: do you wish to see all 102 possibilities? 
> 
>    shows up, can I simply get it to:
>     a) not prompt,
>     b) not list, and
>     c) maybe say "**lotsa completions - not listing**"
> 
> Thanks for the help!

...by adding a key, say `list_lines' or whatever which would allow us
to write shell code that can find out if the listing code would ask
(well, of course we could also check this in C and have a key saying
whether it would ask, but that check is easy and knowing the number of 
lines may be intersting anyway, maybe, hm...). Anyway, with the
relatively new `calclist()'  this would be quite easy to implement
(heck, we could give many different types of information about the
list).

Adam Spiers wrote:

>       d) list the completion groups and the number of matches in each,
>          and then let you choose which group to complete from.
> 
> Selecting completion groups would be a nice feature in any case.
> 
> I might have a go at hacking some of these, but will probably fail and
> have to wait for Sven to get back.  I'm away this w/e myself, however.

This is something I want to achieve (see 12241, at least if I
understand you correctly) with the grouping anhancements to the
completion system discussed lately (keywords: tags, priorities, and
Peter's auto-documentation suggestion).

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH and Re: simulation of dabbrev-expand
  1999-10-11 11:12 PATCH and Re: simulation of dabbrev-expand Sven Wischnowsky
@ 1999-10-11 21:44 ` Adam Spiers
  0 siblings, 0 replies; 6+ messages in thread
From: Adam Spiers @ 1999-10-11 21:44 UTC (permalink / raw)
  To: zsh-workers

Sven Wischnowsky (wischnow@informatik.hu-berlin.de) wrote:
> 
> [ Trying to reduce the number of replies by answering multiple
> messages... ]

Welcome back to the fun :-)

> First, the `design decision' comes from a time when one couldn't
> execute shell code (and look at the current state) during
> completion. Then, I think it is still right to execute the probably
> expansive sorting and uniquifying as seldom as possible -- and in most 
> cases this is really only needed at the end. But I after thinking some 
> more about this, I really think, the solution to do the calculation
> when one of the `nmatches' keys is used is ok.

Yep, this should be fine I guess.

> ...by adding a key, say `list_lines' or whatever which would allow us
> to write shell code that can find out if the listing code would ask
> (well, of course we could also check this in C and have a key saying
> whether it would ask, but that check is easy and knowing the number of 
> lines may be intersting anyway, maybe, hm...). Anyway, with the
> relatively new `calclist()'  this would be quite easy to implement
> (heck, we could give many different types of information about the
> list).

That would be nice too.  Then you could be quite intelligent about
what to display.  For example you might want to specify LIST_MAX as
multiples of the screen length.

> > Selecting completion groups would be a nice feature in any case.
> 
> This is something I want to achieve (see 12241, at least if I
> understand you correctly) with the grouping anhancements to the
> completion system discussed lately (keywords: tags, priorities, and
> Peter's auto-documentation suggestion).

I look forward to it :-)


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH and Re: simulation of dabbrev-expand
  1999-09-23 14:23 Sven Wischnowsky
@ 1999-09-25 14:45 ` Adam Spiers
  0 siblings, 0 replies; 6+ messages in thread
From: Adam Spiers @ 1999-09-25 14:45 UTC (permalink / raw)
  To: zsh-workers

Sven Wischnowsky (wischnow@informatik.hu-berlin.de) wrote:
> [ Last mail from me for the next two weeks... ]

:-(

> >   - Whenever duplicates get removed, it breaks.  It looks like
> >     compstate[nmatches] corresponds with the number of matches
> >     /including/ duplicates, even if some/all duplicates have been
> >     removed.
> 
> At the time where you can look at `compstate', it is `...will be
> removed'. Remember that all the sorting and uniquifying is only done
> after the widget has returned (and for performance reasons we should
> probably only do it then).

Hmm.  This really is a problem.  Anything which wants full control of
menucompletion (and hence implements it itself, e.g. _history_complete_word,
which wants the ability to stop at either end of the matches) will need to
know an accurate compstate[nmatches] straight after the compgen call.
What's the correct solution here?  It would be nice if the C code
implemented loop prevention (it feels kind of hacky doing it all through
messing around with compstate[insert]) but it's difficult to draw the line
between what should be in the C code and what should be shell script ... for
example you might want control over what message (if any) should be
displayed when you hit one of the ends of the list of matches.

Any ideas on what's the Right Way here?

One workaround would be to ensure that whenever compstate[oldlist] is set to
`keep', compstate[nmatches] is set to the size of the old list.  That way,
in the _history_complete_word case, you could recalculate nmatches
accurately on the second call to the widget, which would be good enough.

> Although... we could do it whenever someone 
> looks at `compstate[*nmatches]', set a flag if the list is sorted and
> clear the flag when another match is added. Hm. No time now...

Sounds reasonable, but still feels (IMO) a bit of a workaround forced by the
wrong design decision.  Maybe there's no good solution.  *shrug*


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH and Re: simulation of dabbrev-expand
@ 1999-09-23 14:23 Sven Wischnowsky
  1999-09-25 14:45 ` Adam Spiers
  0 siblings, 1 reply; 6+ messages in thread
From: Sven Wischnowsky @ 1999-09-23 14:23 UTC (permalink / raw)
  To: zsh-workers


[ Last mail from me for the next two weeks... ]

Adam Spiers wrote:

> Sven Wischnowsky (wischnow@informatik.hu-berlin.de) wrote:
> > *But* words
> > from the same line are inserted left-to-right. Since the C-code walks
> > up maybe we should add the words from the end of the line to the
> > beginning.
> > 
> > We should then use the hunk for `zle_tricky.c' below.
> 
> Nice touch.  Reminds me of another feature I've wanted in
> _history_complete_word for ages - would it be possible to modify
> compgen -H to include words from the currently edited line?

Not that simple because we don't have it in parsed form yet. We could
make `get_comp_string()' save them but then we still would only get
the words for the current command and all commands before it.

> Hmm.  I'm still puzzled why compadd -X ... -n '' throws away the old
> list though, since compwidget == lastcompwidget and
> compstate[old_list] is set to keep.

If I understood you correclty: it doesn't. It only looks like that
because of the compstate[insert]='' in _message.

>   - Doesn't cope with numeric arguments yet.
>   - Whenever duplicates get removed, it breaks.  It looks like
>     compstate[nmatches] corresponds with the number of matches
>     /including/ duplicates, even if some/all duplicates have been
>     removed.

At the time where you can look at `compstate', it is `...will be
removed'. Remember that all the sorting and uniquifying is only done
after the widget has returned (and for performance reasons we should
probably only do it then). Although... we could do it whenever someone 
looks at `compstate[*nmatches]', set a flag if the list is sorted and
clear the flag when another match is added. Hm. No time now...

>   - The error message given when unknown keys are bound to the
>     widget doesn't work.  Should I be using zle -R here?

No, `zle -R' will only flash the message (and only one line).
Ignore that? Beep? Use `_message' or similar code?

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de


^ permalink raw reply	[flat|nested] 6+ messages in thread

* PATCH and Re: simulation of dabbrev-expand
  1999-09-23  8:38 Sven Wischnowsky
@ 1999-09-23 13:44 ` Adam Spiers
  0 siblings, 0 replies; 6+ messages in thread
From: Adam Spiers @ 1999-09-23 13:44 UTC (permalink / raw)
  To: zsh-workers

Sven Wischnowsky (wischnow@informatik.hu-berlin.de) wrote:
> 
> First: there was a buglet in `_history_complete_word' which made it
> pretty useless if `history_stop' wasn't set. I hope the patch below
> doesn't interfere with your work...

Nope -- I ended up rewriting the whole damn thing :-)

> Adam Spiers wrote:
> > The current situation is that _history_complete_word crawls through
> > matching history words, oldest first. 
> 
> Now I'm confused. For me it works from the bottom up.

Of course I was talking rubbish again here ... but I'm sure you had
realised that and were just being polite :-)

> *But* words
> from the same line are inserted left-to-right. Since the C-code walks
> up maybe we should add the words from the end of the line to the
> beginning.
> 
> We should then use the hunk for `zle_tricky.c' below.

Nice touch.  Reminds me of another feature I've wanted in
_history_complete_word for ages - would it be possible to modify
compgen -H to include words from the currently edited line?

> > However, I got stuck when handling the history_stop feature.
> > When in verbose mode, history_stop uses _message to indicate that the
> > beginning/end of the history has been reached.  However, unless I've
> > got things really wrong, _message seems to destroy any old list of matches
> > which you might want to keep.  I can't understand why, as it's only
> > essentially a compadd -X ... -n ''. 
> 
> That's right. To be able to display the message we have to add a dummy 
> completion (that empty string) and throw away the old list.

Hmm.  I'm still puzzled why compadd -X ... -n '' throws away the old
list though, since compwidget == lastcompwidget and
compstate[old_list] is set to keep.

> > so that if you hit the oldest match and press M-/ again, it displays
> > this message but keeps the oldest match (should I need a
> > compstate[insert]=1 again, or is it enough to have done that the first
> > time the oldest match was displayed?), and if you switch to M-, it
> > will keep this old_list again and start moving in the opposite
> > direction.
> 
> If I were to implement that, I would use two (global) parameters set
> when the end is reached with `verbose' (or always when the end is
> reached). Any of the two ends, that is. The first parameter just says
> that the end was reached (and could say which end if that is
> needed). The other one would contain the value of `BUFFER' at the time 
> the end was reached. Now if you invoke the widget for the other
> direction when an end was reached (the first parameter sayeth so), you 
> compare the second parameter with the current value of `BUFFER'. If
> they are equal, the thing was just invoked (hm, maybe we shouldn't
> compare `BUFFER', maybe we should check `LASTWIDGET'; or both; and if
> you use the numeric argument thing, the latter may also need to have
> to take that into its calculation). Anyway, the test would say that we 
> just reached one of the ends by the history_complete function for the
> other direction and if we find that out, we just call completion again 
> (that `compgen') and then use `compstate[insert]=-1' (or `...=1' for
> the other direction). Getting words to complete from the history
> should be fast enough so that the user doesn't really notice that a
> new list was created. For this we would also have to keep the original 
> values of `PREFIX' and `SUFFIX' -- and set them anew in the right
> place.

Yep, a similar approach had occurred to me after my last post on the
subject.

> Anyway, I think it can be done...

It can and it has :-)  Here's a full replacement for
Completion/Commands/_history_complete_word.

-------- 8< -------- 8< --------
#compdef -k complete-word \e/ \e,
#
# Complete words from the history
#
# by Adam Spiers, with help gratefully received from
# Sven Wischnowsky and Bart Schaefer
#
# Available configuration keys:
#
#   history_list -- display lists of available matches
#   history_stop -- prevent looping at beginning and end of matches
#                   during menu-completion
#   history_sort -- sort matches lexically (default is to sort by age)
#   history_remove_all_dups --
#                   remove /all/ duplicate matches rather than just
#                   consecutives
#

_history_complete_word () {
  local expl direction

  case "$KEYS" in 
    '^[,')  direction='newer'
            ;;
    '^[/')  direction='older'
            ;;
        *)  print <<EOF
The keypress \`$KEYS\' was not understood by _history_complete_word.
You must alter _history_complete_word if you want to bind it to keys
other than the defaults, so that it knows which direction the key
should move in the history.
EOF
            return 1
  esac

  [[ -z "$compconfig[history_list]" ]] && compstate[list]=''

  if [[ -n "$compstate[old_list]" && -n "$compconfig[history_stop]" ]]; then
    # array of matches is newest -> oldest (reverse of history order)
    if [[ "$direction" == 'older' ]]; then
      if [[ compstate[old_insert] -eq $_hist_menu_length ||
            "$_hist_stop" == 'oldest' ]]; then
        _hist_stop='oldest'
        [[ "$compconfig[history_stop]" = verbose ]] &&
          _message 'beginning of history reached'
      elif [[ "$_hist_stop" == 'newest' ]]; then
        zle -Rc
        _history_complete_word_gen_matches
      else
        compstate[old_list]=keep
        (( compstate[insert] = compstate[old_insert] + 1 ))
      fi
    elif [[ "$direction" == 'newer' ]]; then
      if [[ compstate[old_insert] -eq 1 || "$_hist_stop" == 'newest' ]]; then
        _hist_stop='newest'
        [[ "$compconfig[history_stop]" = verbose ]] &&
          _message 'end of history reached'
      elif [[ "$_hist_stop" == 'oldest' ]]; then
        zle -Rc
        _history_complete_word_gen_matches
      else
        compstate[old_list]=keep
        (( compstate[insert] = compstate[old_insert] - 1 ))
      fi
    fi
  else
    _hist_stop=''
    _hist_old_prefix="$PREFIX"
    _history_complete_word_gen_matches
  fi

  [[ -n "$compstate[nmatches]" ]]
}

_history_complete_word_gen_matches () {
  if [[ -n "$compconfig[history_list]" ]]; then
    if [[ -n "$compconfig[history_sort]" ]]; then
      _description expl 'history word'
    else
      _description -V expl 'history word'
    fi
  else
    if [[ -n "$compconfig[history_sort]" ]]; then
      expl=()
    else
      expl=('-V' '')
    fi
  fi

  [[ -n "$_hist_stop" ]] && PREFIX="$_hist_old_prefix"

  local rem_dups
  if [[ -n "$compconfig[history_remove_all_dups]" ]]; then
    rem_dups=''
  else
    rem_dups='-1'
  fi

  compgen "$expl[@]" $rem_dups -Q -H 0 ''
  _hist_menu_length="$compstate[nmatches]"

  case "$direction" in 
    newer)  compstate[insert]=$_hist_menu_length
	    [[ -n "$_hist_stop" ]] && (( compstate[insert]-- ))
            ;;
    older)  compstate[insert]=1
	    [[ -n "$_hist_stop" ]] && (( compstate[insert]++ ))
            ;;
  esac

  [[ -n "$_hist_stop" ]] && _hist_stop=''
}

_history_complete_word "$@"
-------- 8< -------- 8< --------

Please have a play with the different options and see how robust it is
(not very, probably!)  Known problems:

  - Doesn't cope with numeric arguments yet.
  - Whenever duplicates get removed, it breaks.  It looks like
    compstate[nmatches] corresponds with the number of matches
    /including/ duplicates, even if some/all duplicates have been
    removed.
  - The error message given when unknown keys are bound to the
    widget doesn't work.  Should I be using zle -R here?

> I hope it helped.

It did.  Thanks!


^ permalink raw reply	[flat|nested] 6+ messages in thread

* PATCH and Re: simulation of dabbrev-expand
@ 1999-09-23  8:38 Sven Wischnowsky
  1999-09-23 13:44 ` Adam Spiers
  0 siblings, 1 reply; 6+ messages in thread
From: Sven Wischnowsky @ 1999-09-23  8:38 UTC (permalink / raw)
  To: zsh-workers


First: there was a buglet in `_history_complete_word' which made it
pretty useless if `history_stop' wasn't set. I hope the patch below
doesn't interfere with your work...

Adam Spiers wrote:

> The current situation is that _history_complete_word crawls through
> matching history words, oldest first. 

Now I'm confused. For me it works from the bottom up. *But* words
from the same line are inserted left-to-right. Since the C-code walks
up maybe we should add the words from the end of the line to the
beginning.

We should then use the hunk for `zle_tricky.c' below.

> This is counter-intuitive,
> impractical, and not what tcsh users would expect; so I've been
> trying to change it to the following setup:
> 
>    M-/  _reverse_history_complete_word
>    M-,  _history_complete_word
> 
> _reverse_history_complete_word startest with the most recent match and
> works back, and vice versa for _history_complete_word.

... but a `_reverse_*' would still be good to have (I thought about
this when I made my last changes to `_h_c_w' but was to lazy...).

> However, I got stuck when handling the history_stop feature.
> When in verbose mode, history_stop uses _message to indicate that the
> beginning/end of the history has been reached.  However, unless I've
> got things really wrong, _message seems to destroy any old list of matches
> which you might want to keep.  I can't understand why, as it's only
> essentially a compadd -X ... -n ''. 

That's right. To be able to display the message we have to add a dummy 
completion (that empty string) and throw away the old list.

> I want to do something like:
> 
>     compstate[old_list]=keep
>     _message 'beginning of history reached'

This can't work and I don't think it should. Since the `messages' are
really only explanation string they belong to their own list and
should only be displayed when that list is used.

> so that if you hit the oldest match and press M-/ again, it displays
> this message but keeps the oldest match (should I need a
> compstate[insert]=1 again, or is it enough to have done that the first
> time the oldest match was displayed?), and if you switch to M-, it
> will keep this old_list again and start moving in the opposite
> direction.

If I were to implement that, I would use two (global) parameters set
when the end is reached with `verbose' (or always when the end is
reached). Any of the two ends, that is. The first parameter just says
that the end was reached (and could say which end if that is
needed). The other one would contain the value of `BUFFER' at the time 
the end was reached. Now if you invoke the widget for the other
direction when an end was reached (the first parameter sayeth so), you 
compare the second parameter with the current value of `BUFFER'. If
they are equal, the thing was just invoked (hm, maybe we shouldn't
compare `BUFFER', maybe we should check `LASTWIDGET'; or both; and if
you use the numeric argument thing, the latter may also need to have
to take that into its calculation). Anyway, the test would say that we 
just reached one of the ends by the history_complete function for the
other direction and if we find that out, we just call completion again 
(that `compgen') and then use `compstate[insert]=-1' (or `...=1' for
the other direction). Getting words to complete from the history
should be fast enough so that the user doesn't really notice that a
new list was created. For this we would also have to keep the original 
values of `PREFIX' and `SUFFIX' -- and set them anew in the right
place.

Anyway, I think it can be done...

> Finally, should forward and reverse motion both be handled by the same
> widget?

Shrug, dunno. The builtin widgets always have two forms and use the
numeric argument (and use the other direction if that is negative)...

> If you can help with the above problem then with luck I'll be able to
> save you the hassle of rewriting all the code.  Thanks!

I hope it helped.

Bye
 Sven

diff -u oldcompletion/Commands/_history_complete_word Completion/Commands/_history_complete_word
--- oldcompletion/Commands/_history_complete_word	Tue Sep 21 10:26:40 1999
+++ Completion/Commands/_history_complete_word	Thu Sep 23 10:14:42 1999
@@ -23,8 +23,8 @@
     _description -V expl 'history word'
   fi
   compgen "$expl[@]" -Q -H 0 ''
-  compstate[insert]=1
   if [[ -n "$compconfig[history_stop]" ]]; then
+    compstate[insert]=1
     _hist_menu_length="$compstate[nmatches]"
     _hist_menu_end=''
   fi
diff -u os/Zle/zle_tricky.c Src/Zle/zle_tricky.c
--- os/Zle/zle_tricky.c	Thu Sep 23 10:19:05 1999
+++ Src/Zle/zle_tricky.c	Thu Sep 23 10:21:09 1999
@@ -6932,7 +6932,7 @@
 	/* Now search the history. */
 	while (n-- && he) {
 	    int iwords;
-	    for (iwords = 0; iwords < he->nwords; iwords++) {
+	    for (iwords = he->nwords - 1; iwords >= 0; iwords--) {
 		h = he->text + he->words[iwords*2];
 		e = he->text + he->words[iwords*2+1];
 		hpatsav = *e;

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~1999-10-11 21:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
1999-10-11 11:12 PATCH and Re: simulation of dabbrev-expand Sven Wischnowsky
1999-10-11 21:44 ` Adam Spiers
  -- strict thread matches above, loose matches on Subject: below --
1999-09-23 14:23 Sven Wischnowsky
1999-09-25 14:45 ` Adam Spiers
1999-09-23  8:38 Sven Wischnowsky
1999-09-23 13:44 ` Adam Spiers

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).