Hi

Thanks for the diff,

I think I'd prefer to see a change to add a vi-style next-word now and the
nonspace stuff separately, if that is possible. If it is going to be too
time-consuming don't worry.

> Index: mode-key.c
> ===================================================================
> --- mode-key.c.orig
> +++ mode-key.c
> @@ -83,12 +83,16 @@
>       { MODEKEYCOPY_COPYSELECTION, "copy-selection" },
>       { MODEKEYCOPY_DOWN, "cursor-down" },
>       { MODEKEYCOPY_ENDOFLINE, "end-of-line" },
> +     { MODEKEYCOPY_ENDOFNONSPACEWORD, "end-of-nonspace-word" },

I don't like the use of "nonspace" in these commands, but I haven't thought of
better names so far.

> +     { MODEKEYCOPY_ENDOFWORD, "end-of-word" },
>       { MODEKEYCOPY_GOTOLINE, "goto-line" },
>       { MODEKEYCOPY_LEFT, "cursor-left" },
>       { MODEKEYCOPY_MIDDLELINE, "middle-line" },
>       { MODEKEYCOPY_NEXTPAGE, "page-down" },
> +     { MODEKEYCOPY_NEXTNONSPACEWORD, "next-nonspace-word" },
>       { MODEKEYCOPY_NEXTWORD, "next-word" },
>       { MODEKEYCOPY_PREVIOUSPAGE, "page-up" },
> +     { MODEKEYCOPY_PREVIOUSNONSPACEWORD, "previous-nonspace-word" },
>       { MODEKEYCOPY_PREVIOUSWORD, "previous-word" },
>       { MODEKEYCOPY_RIGHT, "cursor-right" },
>       { MODEKEYCOPY_SCROLLDOWN, "scroll-down" },
> @@ -164,11 +168,14 @@
>       { '0',                  0, MODEKEYCOPY_STARTOFLINE },
>       { ':',                  0, MODEKEYCOPY_GOTOLINE },
>       { '?',                  0, MODEKEYCOPY_SEARCHUP },
> +     { 'B',                  0, MODEKEYCOPY_PREVIOUSNONSPACEWORD },
> +     { 'E',                  0, MODEKEYCOPY_ENDOFNONSPACEWORD },
>       { 'H',                  0, MODEKEYCOPY_TOPLINE },
>       { 'J',                  0, MODEKEYCOPY_SCROLLDOWN },
>       { 'K',                  0, MODEKEYCOPY_SCROLLUP },
>       { 'L',                  0, MODEKEYCOPY_BOTTOMLINE },
>       { 'M',                  0, MODEKEYCOPY_MIDDLELINE },
> +     { 'W',                  0, MODEKEYCOPY_NEXTNONSPACEWORD },
>       { '\002' /* C-b */,     0, MODEKEYCOPY_PREVIOUSPAGE },
>       { '\003' /* C-c */,     0, MODEKEYCOPY_CANCEL },
>       { '\004' /* C-d */,     0, MODEKEYCOPY_HALFPAGEDOWN },
> @@ -181,6 +188,7 @@
>       { '\r',                 0, MODEKEYCOPY_COPYSELECTION },
>       { '^',                  0, MODEKEYCOPY_BACKTOINDENTATION },
>       { 'b',                  0, MODEKEYCOPY_PREVIOUSWORD },
> +     { 'e',                  0, MODEKEYCOPY_ENDOFWORD },
>       { 'h',                  0, MODEKEYCOPY_LEFT },
>       { 'j',                  0, MODEKEYCOPY_DOWN },
>       { 'k',                  0, MODEKEYCOPY_UP },
> @@ -269,7 +277,7 @@
>       { '\027' /* C-w */,     0, MODEKEYCOPY_COPYSELECTION },
>       { '\033' /* Escape */,  0, MODEKEYCOPY_CANCEL },
>       { 'b' | KEYC_ESCAPE,    0, MODEKEYCOPY_PREVIOUSWORD },
> -     { 'f' | KEYC_ESCAPE,    0, MODEKEYCOPY_NEXTWORD },
> +     { 'f' | KEYC_ESCAPE,    0, MODEKEYCOPY_ENDOFWORD },
>       { 'g',                  0, MODEKEYCOPY_GOTOLINE },
>       { 'm' | KEYC_ESCAPE,    0, MODEKEYCOPY_BACKTOINDENTATION },
>       { 'n',                  0, MODEKEYCOPY_SEARCHAGAIN },
> Index: tmux.h
> ===================================================================
> --- tmux.h.orig
> +++ tmux.h
> @@ -451,14 +451,18 @@
>       MODEKEYCOPY_COPYSELECTION,
>       MODEKEYCOPY_DOWN,
>       MODEKEYCOPY_ENDOFLINE,
> +     MODEKEYCOPY_ENDOFNONSPACEWORD,
> +     MODEKEYCOPY_ENDOFWORD,
>       MODEKEYCOPY_GOTOLINE,
>       MODEKEYCOPY_HALFPAGEDOWN,
>       MODEKEYCOPY_HALFPAGEUP,
>       MODEKEYCOPY_LEFT,
>       MODEKEYCOPY_MIDDLELINE,
>       MODEKEYCOPY_NEXTPAGE,
> +     MODEKEYCOPY_NEXTNONSPACEWORD,
>       MODEKEYCOPY_NEXTWORD,
>       MODEKEYCOPY_PREVIOUSPAGE,
> +     MODEKEYCOPY_PREVIOUSNONSPACEWORD,
>       MODEKEYCOPY_PREVIOUSWORD,
>       MODEKEYCOPY_RIGHT,
>       MODEKEYCOPY_SCROLLDOWN,
> Index: window-copy.c
> ===================================================================
> --- window-copy.c.orig
> +++ window-copy.c
> @@ -54,7 +54,7 @@
>  void window_copy_copy_selection(struct window_pane *, struct client *);
>  void window_copy_copy_line(
>           struct window_pane *, char **, size_t *, u_int, u_int, u_int);
> -int  window_copy_is_space(struct window_pane *, u_int, u_int);
> +int  window_copy_strchr(struct window_pane *, u_int, u_int, const char *);
>  u_int        window_copy_find_length(struct window_pane *, u_int);
>  void window_copy_cursor_start_of_line(struct window_pane *);
>  void window_copy_cursor_back_to_indentation(struct window_pane *);
> @@ -63,8 +63,8 @@
>  void window_copy_cursor_right(struct window_pane *);
>  void window_copy_cursor_up(struct window_pane *, int);
>  void window_copy_cursor_down(struct window_pane *, int);
> -void window_copy_cursor_next_word(struct window_pane *);
> -void window_copy_cursor_previous_word(struct window_pane *);
> +void window_copy_cursor_next_word(struct window_pane *, const char *, int);
> +void window_copy_cursor_previous_word(struct window_pane *, const char *);
>  void window_copy_scroll_up(struct window_pane *, u_int);
>  void window_copy_scroll_down(struct window_pane *, u_int);
>  
> @@ -84,6 +84,11 @@
>       WINDOW_COPY_GOTOLINE,
>  };
>  
> +enum word_search_mode {
> +     WORD_SEARCH_START_OF_WORD,
> +     WORD_SEARCH_END_OF_WORD,
> +};

I'd make this something like enum window_copy_search_mode,
WINDOW_COPY_WORDSTART, WINDOW_COPY_WORDEND.

> +
>  struct window_copy_mode_data {
>       struct screen   screen;
>  
> @@ -108,6 +113,8 @@
>       char           *searchstr;
>  };
>  
> +const char *window_copy_word_delims = " -_@";

I think this is still only used in one function so it can remain local,
although it could be static in there.

> +
>  struct screen *
>  window_copy_init(struct window_pane *wp)
>  {
> @@ -319,11 +326,27 @@
>       case MODEKEYCOPY_ENDOFLINE:
>               window_copy_cursor_end_of_line(wp);
>               break;
> +     case MODEKEYCOPY_ENDOFNONSPACEWORD:
> +             window_copy_cursor_next_word(
> +                 wp, " ", WORD_SEARCH_END_OF_WORD);
> +             break;
> +     case MODEKEYCOPY_ENDOFWORD:
> +             window_copy_cursor_next_word(
> +                 wp, window_copy_word_delims, WORD_SEARCH_END_OF_WORD);
> +             break;
> +     case MODEKEYCOPY_NEXTNONSPACEWORD:
> +             window_copy_cursor_next_word(
> +                 wp, " ", WORD_SEARCH_START_OF_WORD);
> +             break;
>       case MODEKEYCOPY_NEXTWORD:
> -             window_copy_cursor_next_word(wp);
> +             window_copy_cursor_next_word(
> +                 wp, window_copy_word_delims, WORD_SEARCH_START_OF_WORD);
> +             break;
> +     case MODEKEYCOPY_PREVIOUSNONSPACEWORD:
> +             window_copy_cursor_previous_word(wp, " ");
>               break;
>       case MODEKEYCOPY_PREVIOUSWORD:
> -             window_copy_cursor_previous_word(wp);
> +             window_copy_cursor_previous_word(wp, window_copy_word_delims);
>               break;
>       case MODEKEYCOPY_SEARCHUP:
>               data->inputtype = WINDOW_COPY_SEARCHUP;
> @@ -945,17 +968,17 @@
>  }
>  
>  int
> -window_copy_is_space(struct window_pane *wp, u_int px, u_int py)
> +window_copy_strchr(
> +    struct window_pane *wp, u_int px, u_int py, const char *chars)
>  {
>       const struct grid_cell  *gc;
> -     const char              *spaces = " -_@";
>  
>       gc = grid_peek_cell(wp->base.grid, px, py);
>       if (gc->flags & (GRID_FLAG_PADDING|GRID_FLAG_UTF8))
>               return (0);
>       if (gc->data == 0x00 || gc->data == 0x7f)
>               return (0);
> -     return (strchr(spaces, gc->data) != NULL);
> +     return (strchr(chars, gc->data) != NULL);
>  }
>  
>  u_int
> @@ -1005,10 +1028,6 @@
>       py = screen_hsize(&wp->base) + data->cy - data->oy;
>       xx = window_copy_find_length(wp, py);
>  
> -     /*
> -      * Don't use window_copy_is_space because that treats some word
> -      * delimiters as spaces.
> -      */
>       while (px < xx) {
>               gc = grid_peek_cell(wp->base.grid, px, py);
>               if (gc->flags & GRID_FLAG_UTF8)
> @@ -1133,26 +1152,31 @@
>  }
>  
>  void
> -window_copy_cursor_next_word(struct window_pane *wp)
> +window_copy_cursor_next_word(
> +    struct window_pane *wp, const char *nonword, int mode)
>  {
>       struct window_copy_mode_data    *data = wp->modedata;
>       struct screen                   *s = &data->screen;
> -     u_int                            px, py, xx, skip;
> +     u_int                            px, py, xx, search_for_nonword;
> +     u_int                            stop_at_end;
>  
>       px = data->cx;
>       py = screen_hsize(&wp->base) + data->cy - data->oy;
>       xx = window_copy_find_length(wp, py);
>  
> -     skip = 1;
> +     stop_at_end = (mode == WORD_SEARCH_END_OF_WORD);
> +     search_for_nonword = 1;
>       if (px < xx) {
>               /* If currently on a space, skip space. */
> -             if (window_copy_is_space(wp, px, py))
> -                     skip = 0;
> +             if (window_copy_strchr(wp, px, py, nonword))
> +                     search_for_nonword = 0;
>       } else
> -             skip = 0;
> +             search_for_nonword = 0;
>       for (;;) {
> +             u_int           is_space;
> +
>               if (px >= xx) {
> -                     if (skip) {
> +                     if (search_for_nonword && stop_at_end) {
>                               px = xx;
>                               break;
>                       }
> @@ -1172,14 +1196,17 @@
>                       }
>               }
>  
> -             if (skip) {
> -                     /* Currently skipping non-space (until space). */
> -                     if (window_copy_is_space(wp, px, py))
> -                             break;
> -             } else {
> -                     /* Currently skipping space (until non-space). */
> -                     if (!window_copy_is_space(wp, px, py))
> -                             skip = 1;
> +             is_space = window_copy_strchr(wp, px, py, nonword);
> +             if ((is_space && search_for_nonword && stop_at_end)
> +                 || (!is_space && !search_for_nonword && !stop_at_end)) {
> +                     /* If we've reached the spot we're looking for, we're
> +                      * done looking. Yay tautologies! :) */
> +                     break;
> +             } else if (is_space == search_for_nonword) {
> +                     /* If we're not stopping yet, but we've just
> +                      * reached a word-boundary, then toggle
> +                      * search_for_nonword to indicate that. */
> +                     search_for_nonword = !search_for_nonword;
>               }

I wonder if there is a way to do this that is easier to read than adding extra
flags, although the current code isn't perfect. We got rid of the skip flag in
window_copy_cursor_previous_word maybe it isn't still necessary in here.

If you can break the diff into two I'll have a closer look at this.

>  
>               px++;
> @@ -1193,7 +1220,7 @@
>  
>  /* Move to the previous place where a word begins. */
>  void
> -window_copy_cursor_previous_word(struct window_pane *wp)
> +window_copy_cursor_previous_word(struct window_pane *wp, const char *nonword)
>  {
>       struct window_copy_mode_data    *data = wp->modedata;
>       u_int                            px, py;
> @@ -1205,7 +1232,7 @@
>       for (;;) {
>               if (px > 0) {
>                       px--;
> -                     if (!window_copy_is_space(wp, px, py))
> +                     if (!window_copy_strchr(wp, px, py, nonword))
>                               break;
>               } else {
>                       if (data->cy == 0 &&
> @@ -1221,7 +1248,7 @@
>       }
>  
>       /* Move back to the beginning of this word. */
> -     while (px > 0 && !window_copy_is_space(wp, px - 1, py))
> +     while (px > 0 && !window_copy_strchr(wp, px - 1, py, nonword))
>               px--;
>  
>  out:
> Index: tmux.1
> ===================================================================
> --- tmux.1.orig
> +++ tmux.1
> @@ -535,7 +535,7 @@
>  .Ic mode-keys
>  option).
>  The following keys are supported as appropriate for the mode:
> -.Bl -column "FunctionXXXXXXXXXXXX" "viXXXXXXXXXX" "emacs" -offset indent
> +.Bl -column "FunctionXXXXXXXXXXXXXX" "viXXXXXXXXXX" "emacs" -offset indent
>  .It Sy "Function" Ta Sy "vi" Ta Sy "emacs"
>  .It Li "Back to indentation" Ta "^" Ta "M-m"
>  .It Li "Clear selection" Ta "Escape" Ta "C-g"
> @@ -550,14 +550,18 @@
>  .It Li "Delete entire line" Ta "d" Ta "C-u"
>  .It Li "Delete to end of line" Ta "D" Ta "C-k"
>  .It Li "End of line" Ta "$" Ta "C-e"
> +.It Li "End of non-space word" Ta "E" Ta ""
> +.It Li "End of word" Ta "e" Ta "M-f"
>  .It Li "Goto line" Ta ":" Ta "g"
>  .It Li "Half page down" Ta "C-d" Ta "M-Down"
>  .It Li "Half page up" Ta "C-u" Ta "M-Up"
>  .It Li "Next page" Ta "C-f" Ta "Page down"
> -.It Li "Next word" Ta "w" Ta "M-f"
> +.It Li "Next non-space word" Ta "W" Ta ""
> +.It Li "Next word" Ta "w" Ta ""
>  .It Li "Paste buffer" Ta "p" Ta "C-y"
>  .It Li "Previous page" Ta "C-b" Ta "Page up"
>  .It Li "Previous word" Ta "b" Ta "M-b"
> +.It Li "Previous non-space word" Ta "B" Ta ""
>  .It Li "Quit mode" Ta "q" Ta "Escape"
>  .It Li "Scroll down" Ta "C-Down or C-e" Ta "C-Down"
>  .It Li "Scroll up" Ta "C-Up or C-y" Ta "C-Up"
> 



On Sun, Jan 24, 2010 at 07:14:16PM -0800, Micah Cowan wrote:
> = Background =
> 
> I've noticed that the "next-word" copy-mode command, bound to "M-f" in
> emacs-mode and to "w" in vi-mode, behaves more like vi's "e", which
> skips to the next word-ending, than its "w", which skips to the next
> word-beginning.
> 
> Also, the vi copy mode currently lacks vi's uppercased equivalents to b,
> w and e (B, W, and E), which behave the same, but treat all non-space
> characters as "word" characters.
> 
> = Functional changes =
> 
> This patch remedies this situation, by renaming the current "next-word"
> as "end-of-word", and using "next-word" to mean "go to the next start of
> a word". Emacs' "M-f" is now bound to "end-of-word" (behaves exactly as
> it has), vi's "e", rather than "w", is now bound to "end-of-word", and
> "w" is bound to "next-word" (as it was, but the behavior of that command
> is now different). The new "next-word" has nothing bound to it for emacs
> (and Emacs has no equivalent word-movement; when I've wanted to move to
> the beginning of the next word in Emacs, I've typically typed M-f M-f
> M-b to get there).
> 
> In addition, I've also introduced three new copy-mode commands,
> "end-of-nonspace-word", "next-nonspace-word", and
> "previous-nonspace-word". These behave the same as their non-"nonspace"
> versions, but they also skip over '-', '_', and '@' characters as "word"
>  characters. These are bound to E, W, and B, respectively. None of these
> functions are bound for the emacs copy mode.
> 
> Note that there is _no_ change whatsoever for emacs-mode, except for a
> change in command-name. If anyone had their own binding to "next-word",
> that binding's functionality has changed (slightly).
> 
> = Patch notes =

> 
> The window_copy_is_space() function has been replaced by
> window_copy_strchr(), which is exactly the same except that instead of
> hardcoding a value for what "spaces" are (several of which could not
> really be called "spaces"), it takes a string parameter and simply
> returns whether the designated cell matches any of the characters
> supplied. " " is used for the "nonspace" variants, " -_@" otherwise.

Not wild about the name, since it isn't doing the same thing as strchr, it only
checks one cell.

> The functions window_copy_cursor_next_word() and
> window_copy_cursor_previous() have both been given a string parameter to
> pass to window_copy_strchr() to determine whether they've encountered a
> "space" or not.
> 
> window_copy_cursor_next_word() also has a new "mode" parameter, which
> tells it whether it's searching for the beginning or end of a word.
> Thus, this function is employed for both "next-(nonspace-)word" and
> "end-of-(nonspace-)word".
>
> = Additional thoughts =
> 
> The current " -_@" set of delimiters differ from what vi typically
> considers to be word delimiters. Vi considers letters, digits and
> underscores to be words, and all other characters to be delimiters.
> Notably, this excludes periods, commas, hyphens, and all other
> punctuation as word characters, and _includes_ the underscore as a
> "word" character. Vi also considers a string of non-word, non-space
> characters (e.g., "..." or "----") to be a word.

Yes I want to add some more as well at some point, can't remember exactly what
offhand but I've noticed it does the wrong thing sometimes.

> 
> Since the current default set of delimiters is only a few characters
> more than just the space itself, and since all our word-movement
> functions now take an argument to specify the
> set-of-delimiting-characters, I did toy with the idea of adding commands
> like "vi-next-word", "vi-previous-word", etc, to pass a larger set of
> "non-word" characters in, but probably much better would be to let the
> user specify the set of characters to be considered "nonword"
> characters. Chances are someone will ask for that anyway, and in that
> case, vi-copy and emacs-copy could/should use that same set of
> characters. (Would you accept a patch for such a feature, Nicholas?)

Yes it can be a server or window option, dunno which.

> 
> -- 
> Micah J. Cowan
> http://micah.cowan.name/


> ------------------------------------------------------------------------------
> Throughout its 18-year history, RSA Conference consistently attracts the
> world's best and brightest in the field, creating opportunities for Conference
> attendees to learn about information security's most important issues through
> interactions with peers, luminaries and emerging and established companies.
> http://p.sf.net/sfu/rsaconf-dev2dev

> _______________________________________________
> tmux-users mailing list
> tmux-users@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tmux-users


------------------------------------------------------------------------------
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
_______________________________________________
tmux-users mailing list
tmux-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tmux-users

Reply via email to