Re: vi(1): documenting :s

2017-06-20 Thread Jason McIntyre
On Tue, Jun 20, 2017 at 01:30:35AM -0600, Anthony J. Bentley wrote:
> Hi,
> 
> Jason McIntyre writes:
> > shouldn;t it be that we should show the suspend command as
> > 
> > sus[pend]
> > 
> > the shortest "s" matches "substitute", right. so we show it as
> > 
> > s[ubstitute]
> > 
> > i cannot find any text that describes what "su" *should* match though,
> > so i'm not sure. logically i'd expect it to match "substitute", since
> > that is first alphabetically. that's why i think it should be sus[pend],
> > not sub[stitute].
> > 
> > i don;t have any other versions of vi to compare how other systems do
> > this.
> 
> Turns out there is no consistency anywhere.
> 
> Original vi implements:
> s
> su[spend]
> sub[stitute]
> 
> nvi (ours and others):
> s
> su[spend]
> 
> elvis:
> s[ubstitute] (it documents sus[pend] but doesn't actually implement it)
> 
> vim:
> s[ubstitute]
> sus[pend]
> 
> It looks like nvi tried to mimic original vi at the beginning, but
> broke the functionality sometime between 1994 and 1996, and nobody
> noticed over the next 20 years.
> 
> Seeing as POSIX is unclear and everybody's inconsistent anyway, I'm
> strongly inclined to just leave things as they are, commit the original
> diff that documents the current reality, and leave it at that.
> 

morning.

i think that makes sense too.

jmc



Re: vi(1): documenting :s

2017-06-20 Thread Anthony J. Bentley
Hi,

Jason McIntyre writes:
> shouldn;t it be that we should show the suspend command as
> 
>   sus[pend]
> 
> the shortest "s" matches "substitute", right. so we show it as
> 
>   s[ubstitute]
> 
> i cannot find any text that describes what "su" *should* match though,
> so i'm not sure. logically i'd expect it to match "substitute", since
> that is first alphabetically. that's why i think it should be sus[pend],
> not sub[stitute].
> 
> i don;t have any other versions of vi to compare how other systems do
> this.

Turns out there is no consistency anywhere.

Original vi implements:
s
su[spend]
sub[stitute]

nvi (ours and others):
s
su[spend]

elvis:
s[ubstitute] (it documents sus[pend] but doesn't actually implement it)

vim:
s[ubstitute]
sus[pend]

It looks like nvi tried to mimic original vi at the beginning, but
broke the functionality sometime between 1994 and 1996, and nobody
noticed over the next 20 years.

Seeing as POSIX is unclear and everybody's inconsistent anyway, I'm
strongly inclined to just leave things as they are, commit the original
diff that documents the current reality, and leave it at that.

-- 
Anthony J. Bentley



Re: vi(1): documenting :s

2017-06-19 Thread Jason McIntyre
On Mon, Jun 19, 2017 at 03:57:35AM -0600, Anthony J. Bentley wrote:
> Hi,
> 
> Jason McIntyre writes:
> > ok by me. note that posix ex(1) does detail a working [s]ubstitute command,
> > so i'm not sure whether we should support this or not.
> 
> Hm, so it does. I think I would prefer to follow POSIX in this case.
> Here's a diff to allow "substitute" to work.
> 
> Annoyingly, there's an ambiguity in the POSIX synopsis (and ours).
> 
>  [2addr] s[ubstitute][/pattern/repl/[options][count][flags]]
> 
> This implies that "su" could expand to "substitute". But it expands (and
> should expand) to "suspend" instead. Since in our vi the first match
> always wins, the simplest way to implement this is to use separate ex
> commands:
> 
>  [2addr] s[/pattern/repl/[options][count][flags]]
>  [2addr] sub[stitute][/pattern/repl/[options][count][flags]]
> 
> There are already plenty of duplicate commands (e.g., "#"/"nu",
> "cd"/"chd", "co"/"t"...) so I think it's an acceptable approach.
> 

evening.

shouldn;t it be that we should show the suspend command as

sus[pend]

the shortest "s" matches "substitute", right. so we show it as

s[ubstitute]

i cannot find any text that describes what "su" *should* match though,
so i'm not sure. logically i'd expect it to match "substitute", since
that is first alphabetically. that's why i think it should be sus[pend],
not sub[stitute].

i don;t have any other versions of vi to compare how other systems do
this.

jmc

> Index: docs/USD.doc/vi.man/vi.1
> ===
> RCS file: /cvs/src/usr.bin/vi/docs/USD.doc/vi.man/vi.1,v
> retrieving revision 1.67
> diff -u -p -r1.67 vi.1
> --- docs/USD.doc/vi.man/vi.1  15 Jun 2017 06:44:47 -  1.67
> +++ docs/USD.doc/vi.man/vi.1  19 Jun 2017 09:19:23 -
> @@ -356,7 +356,7 @@ matches the end of the word.
>  .It
>  .Sq ~
>  matches the replacement part of the last
> -.Cm substitute
> +.Cm s
>  command.
>  .El
>  .Sh BUFFERS
> @@ -1996,52 +1996,65 @@ Grow or shrink the current screen.
>  Rewind the argument list.
>  .Pp
>  .It Xo
> -.Cm se Ns Op Cm t
> +.Op Ar range
>  .Sm off
> -.Op option Oo = Oo value Oc Oc \ \&...
> +.Cm s
> +.Oo Cm / Ar pattern Cm / Ar replace Cm /
> +.Op Ar options
> +.Op Ar count
> +.Op Ar flags
> +.Oc
>  .Sm on
> -.Pf \ \& Op nooption ...
> -.Op option? ...
> -.Op Ar all
> -.Xc
> -Display or set editor options.
> -.Pp
> -.It Cm sh Ns Op Cm ell
> -Run a shell program.
> -.Pp
> -.It Xo
> -.Cm so Ns Op Cm urce
> -.Ar file
>  .Xc
> -Read and execute
> -.Nm ex
> -commands from a file.
> -.Pp
>  .It Xo
>  .Op Ar range
> -.Cm s Ns Op Cm ubstitute
>  .Sm off
> -.Op / Ar pattern No / Ar replace  No /
> -.Sm on
> -.Pf \ \& Op Ar options
> +.Cm sub Op Cm stitute
> +.Oo Cm / Ar pattern Cm / Ar replace Cm /
> +.Op Ar options
>  .Op Ar count
>  .Op Ar flags
> +.Sm on
>  .Xc
>  .It Xo
>  .Op Ar range
> +.Sm off
>  .Cm &
>  .Op Ar options
>  .Op Ar count
>  .Op Ar flags
> +.Sm on
>  .Xc
>  .It Xo
>  .Op Ar range
> +.Sm off
>  .Cm ~
>  .Op Ar options
>  .Op Ar count
>  .Op Ar flags
> +.Sm on
>  .Xc
> -Make substitutions.
> +Substitute the regular expression
> +.Ar pattern
> +with
> +.Ar replace .
> +When invoked as
> +.Cm & ,
> +or if
> +.Cm / Ns Ar pattern Ns Cm / Ns Ar replace Ns Cm /
> +is omitted,
> +.Ar pattern
> +and
> +.Ar replace
> +from the most recent
> +.Cm s
> +command are used.
> +.Cm ~
> +behaves like
> +.Cm & ,
> +except the pattern used is the most recent regular expression used by any
> +command.
> +.Pp
>  The
>  .Ar replace
>  field may contain any of the following sequences:
> @@ -2051,13 +2064,13 @@ The text matched by
>  .Ar pattern .
>  .It Sq \(a~
>  The replacement part of the previous
> -.Cm substitute
> +.Cm s
>  command.
>  .It Sq %
>  If this is the entire
>  .Ar replace
>  pattern, the replacement part of the previous
> -.Cm substitute
> +.Cm s
>  command.
>  .It Sq \e#
>  Where
> @@ -2082,6 +2095,40 @@ to be converted to uppercase.
>  Causes the next character to be converted to uppercase.
>  .El
>  .Pp
> +The
> +.Ar options
> +field may contain any of the following characters:
> +.Bl -tag -width Ds
> +.It Sq c
> +Prompt for confirmation before each replacement is done.
> +.It Sq g
> +Replace all instances of
> +.Ar pattern
> +in a line, not just the first.
> +.El
> +.Pp
> +.It Xo
> +.Cm se Ns Op Cm t
> +.Sm off
> +.Op option Oo = Oo value Oc Oc \ \&...
> +.Sm on
> +.Pf \ \& Op nooption ...
> +.Op option? ...
> +.Op Ar all
> +.Xc
> +Display or set editor options.
> +.Pp
> +.It Cm sh Ns Op Cm ell
> +Run a shell program.
> +.Pp
> +.It Xo
> +.Cm so Ns Op Cm urce
> +.Ar file
> +.Xc
> +Read and execute
> +.Nm ex
> +commands from a file.
> +.Pp
>  .It Xo
>  .Cm su Ns Op Cm spend Ns
>  .Op Cm !\&
> @@ -2291,7 +2338,9 @@ Remember the values of the
>  and
>  .Sq g
>  suffixes to the
> -.Cm substitute
> +.Cm s , &
> +and
> +.Cm ~
>  commands, instead of initializing them as unset for each new command.
>  .It Cm escapetime Bq 1
>  

Re: vi(1): documenting :s

2017-06-19 Thread Anthony J. Bentley
Theo Buehler writes:
> This looks like a reasonable approach and it appears to work. When I
> looked at this after jmc's question, I was scared off by the comment
> 
> >   * Adding new commands starting with 's' may break the substitute command 
> code
> >   * in ex_cmd() (the ex parser).  Read through the comments there, first.
> 
> which is also visible in your diff. I'm not entirely sure what this
> is talking about. Thus, only a hesitant ok for the C-part of your patch.

That comment is referring to the fact that, e.g., "sg" is a legal
command equivalent to ""; see ex/ex.c:430. Adding a command starting
with "sub" won't affect this, because there's no 'u' flag (plus there's
a command starting with "su" already).

-- 
Anthony J. Bentley



Re: vi(1): documenting :s

2017-06-19 Thread Theo Buehler
On Mon, Jun 19, 2017 at 03:57:35AM -0600, Anthony J. Bentley wrote:
> Hi,
> 
> Jason McIntyre writes:
> > ok by me. note that posix ex(1) does detail a working [s]ubstitute command,
> > so i'm not sure whether we should support this or not.
> 
> Hm, so it does. I think I would prefer to follow POSIX in this case.
> Here's a diff to allow "substitute" to work.
> 
> Annoyingly, there's an ambiguity in the POSIX synopsis (and ours).
> 
>  [2addr] s[ubstitute][/pattern/repl/[options][count][flags]]
> 
> This implies that "su" could expand to "substitute". But it expands (and
> should expand) to "suspend" instead. Since in our vi the first match
> always wins, the simplest way to implement this is to use separate ex
> commands:
> 
>  [2addr] s[/pattern/repl/[options][count][flags]]
>  [2addr] sub[stitute][/pattern/repl/[options][count][flags]]
> 
> There are already plenty of duplicate commands (e.g., "#"/"nu",
> "cd"/"chd", "co"/"t"...) so I think it's an acceptable approach.

This looks like a reasonable approach and it appears to work. When I
looked at this after jmc's question, I was scared off by the comment

>   * Adding new commands starting with 's' may break the substitute command 
> code
>   * in ex_cmd() (the ex parser).  Read through the comments there, first.

which is also visible in your diff. I'm not entirely sure what this
is talking about. Thus, only a hesitant ok for the C-part of your patch.

I'll leave it to jmc to verify the vi.1 part in full detail, but it does
look good to me.

Thanks!



Re: vi(1): documenting :s

2017-06-19 Thread Anthony J. Bentley
Hi,

Jason McIntyre writes:
> ok by me. note that posix ex(1) does detail a working [s]ubstitute command,
> so i'm not sure whether we should support this or not.

Hm, so it does. I think I would prefer to follow POSIX in this case.
Here's a diff to allow "substitute" to work.

Annoyingly, there's an ambiguity in the POSIX synopsis (and ours).

 [2addr] s[ubstitute][/pattern/repl/[options][count][flags]]

This implies that "su" could expand to "substitute". But it expands (and
should expand) to "suspend" instead. Since in our vi the first match
always wins, the simplest way to implement this is to use separate ex
commands:

 [2addr] s[/pattern/repl/[options][count][flags]]
 [2addr] sub[stitute][/pattern/repl/[options][count][flags]]

There are already plenty of duplicate commands (e.g., "#"/"nu",
"cd"/"chd", "co"/"t"...) so I think it's an acceptable approach.

Index: docs/USD.doc/vi.man/vi.1
===
RCS file: /cvs/src/usr.bin/vi/docs/USD.doc/vi.man/vi.1,v
retrieving revision 1.67
diff -u -p -r1.67 vi.1
--- docs/USD.doc/vi.man/vi.115 Jun 2017 06:44:47 -  1.67
+++ docs/USD.doc/vi.man/vi.119 Jun 2017 09:19:23 -
@@ -356,7 +356,7 @@ matches the end of the word.
 .It
 .Sq ~
 matches the replacement part of the last
-.Cm substitute
+.Cm s
 command.
 .El
 .Sh BUFFERS
@@ -1996,52 +1996,65 @@ Grow or shrink the current screen.
 Rewind the argument list.
 .Pp
 .It Xo
-.Cm se Ns Op Cm t
+.Op Ar range
 .Sm off
-.Op option Oo = Oo value Oc Oc \ \&...
+.Cm s
+.Oo Cm / Ar pattern Cm / Ar replace Cm /
+.Op Ar options
+.Op Ar count
+.Op Ar flags
+.Oc
 .Sm on
-.Pf \ \& Op nooption ...
-.Op option? ...
-.Op Ar all
-.Xc
-Display or set editor options.
-.Pp
-.It Cm sh Ns Op Cm ell
-Run a shell program.
-.Pp
-.It Xo
-.Cm so Ns Op Cm urce
-.Ar file
 .Xc
-Read and execute
-.Nm ex
-commands from a file.
-.Pp
 .It Xo
 .Op Ar range
-.Cm s Ns Op Cm ubstitute
 .Sm off
-.Op / Ar pattern No / Ar replace  No /
-.Sm on
-.Pf \ \& Op Ar options
+.Cm sub Op Cm stitute
+.Oo Cm / Ar pattern Cm / Ar replace Cm /
+.Op Ar options
 .Op Ar count
 .Op Ar flags
+.Sm on
 .Xc
 .It Xo
 .Op Ar range
+.Sm off
 .Cm &
 .Op Ar options
 .Op Ar count
 .Op Ar flags
+.Sm on
 .Xc
 .It Xo
 .Op Ar range
+.Sm off
 .Cm ~
 .Op Ar options
 .Op Ar count
 .Op Ar flags
+.Sm on
 .Xc
-Make substitutions.
+Substitute the regular expression
+.Ar pattern
+with
+.Ar replace .
+When invoked as
+.Cm & ,
+or if
+.Cm / Ns Ar pattern Ns Cm / Ns Ar replace Ns Cm /
+is omitted,
+.Ar pattern
+and
+.Ar replace
+from the most recent
+.Cm s
+command are used.
+.Cm ~
+behaves like
+.Cm & ,
+except the pattern used is the most recent regular expression used by any
+command.
+.Pp
 The
 .Ar replace
 field may contain any of the following sequences:
@@ -2051,13 +2064,13 @@ The text matched by
 .Ar pattern .
 .It Sq \(a~
 The replacement part of the previous
-.Cm substitute
+.Cm s
 command.
 .It Sq %
 If this is the entire
 .Ar replace
 pattern, the replacement part of the previous
-.Cm substitute
+.Cm s
 command.
 .It Sq \e#
 Where
@@ -2082,6 +2095,40 @@ to be converted to uppercase.
 Causes the next character to be converted to uppercase.
 .El
 .Pp
+The
+.Ar options
+field may contain any of the following characters:
+.Bl -tag -width Ds
+.It Sq c
+Prompt for confirmation before each replacement is done.
+.It Sq g
+Replace all instances of
+.Ar pattern
+in a line, not just the first.
+.El
+.Pp
+.It Xo
+.Cm se Ns Op Cm t
+.Sm off
+.Op option Oo = Oo value Oc Oc \ \&...
+.Sm on
+.Pf \ \& Op nooption ...
+.Op option? ...
+.Op Ar all
+.Xc
+Display or set editor options.
+.Pp
+.It Cm sh Ns Op Cm ell
+Run a shell program.
+.Pp
+.It Xo
+.Cm so Ns Op Cm urce
+.Ar file
+.Xc
+Read and execute
+.Nm ex
+commands from a file.
+.Pp
 .It Xo
 .Cm su Ns Op Cm spend Ns
 .Op Cm !\&
@@ -2291,7 +2338,9 @@ Remember the values of the
 and
 .Sq g
 suffixes to the
-.Cm substitute
+.Cm s , &
+and
+.Cm ~
 commands, instead of initializing them as unset for each new command.
 .It Cm escapetime Bq 1
 The tenths of a second
Index: ex/ex_cmd.c
===
RCS file: /cvs/src/usr.bin/vi/ex/ex_cmd.c,v
retrieving revision 1.10
diff -u -p -r1.10 ex_cmd.c
--- ex/ex_cmd.c 19 Nov 2015 07:53:31 -  1.10
+++ ex/ex_cmd.c 19 Jun 2017 09:19:23 -
@@ -288,8 +288,8 @@ EXCMDLIST const cmds[] = {
  * Adding new commands starting with 's' may break the substitute command code
  * in ex_cmd() (the ex parser).  Read through the comments there, first.
  */
-/* C_SUBSTITUTE */
-   {"s",   ex_s,   E_ADDR2,
+/* C_S */
+   {"s",   ex_s,   E_ADDR2,
"s",
"[line [,line]] s [[/;]RE[/;]repl[/;] [cgr] [count] [#lp]]",
"substitute on lines matching an RE"},
@@ -323,6 +323,11 @@ EXCMDLIST const cmds[] = {
"!",
"su[spend][!]",
"suspend the edit session"},
+/* C_SUBSTITUTE */
+   

Re: vi(1): documenting :s

2017-06-15 Thread Jason McIntyre
On Thu, Jun 15, 2017 at 02:14:46AM -0600, Anthony J. Bentley wrote:
> Hi,
> 
> From vi(1):
> 
>  [range] s[ubstitute] [/pattern/replace/] ??[options] [count] [flags]
>  [range] & [options] [count] [flags]
>  [range] ~ [options] [count] [flags]
>  Make substitutions.  The replace field may contain any of the
>  following sequences:
>  (...snip...)
> 
> There are a couple of issues here.
> 
> First, the command is "s" and only "s"; trying "substitute" results in
> an error.
> 
> Additionally &, ~, and the options field remain unexplained.
> 
> This diff tries to makes things better. ok?
> 

ok by me. note that posix ex(1) does detail a working [s]ubstitute command,
so i'm not sure whether we should support this or not.

if you do make these changes, you may want to move the "s" text up a
little, since these commands are sorted.

jmc

> Index: docs/USD.doc/vi.man/vi.1
> ===
> RCS file: /cvs/src/usr.bin/vi/docs/USD.doc/vi.man/vi.1,v
> retrieving revision 1.67
> diff -u -p -r1.67 vi.1
> --- docs/USD.doc/vi.man/vi.1  15 Jun 2017 06:44:47 -  1.67
> +++ docs/USD.doc/vi.man/vi.1  15 Jun 2017 08:08:49 -
> @@ -356,7 +356,7 @@ matches the end of the word.
>  .It
>  .Sq ~
>  matches the replacement part of the last
> -.Cm substitute
> +.Cm s
>  command.
>  .El
>  .Sh BUFFERS
> @@ -2019,7 +2019,7 @@ commands from a file.
>  .Pp
>  .It Xo
>  .Op Ar range
> -.Cm s Ns Op Cm ubstitute
> +.Cm s
>  .Sm off
>  .Op / Ar pattern No / Ar replace  No /
>  .Sm on
> @@ -2041,7 +2041,27 @@ commands from a file.
>  .Op Ar count
>  .Op Ar flags
>  .Xc
> -Make substitutions.
> +Substitute the regular expression
> +.Ar pattern
> +with
> +.Ar replace .
> +When invoked as
> +.Cm & ,
> +or if
> +.Bq Ns / Ns Ar pattern Ns / Ns Ar replace Ns /
> +is omitted,
> +.Ar pattern
> +and
> +.Ar replace
> +from the most recent
> +.Cm s
> +command are used.
> +.Cm ~
> +behaves like
> +.Cm & ,
> +except the pattern used is the most recent regular expression used by any
> +command.
> +.Pp
>  The
>  .Ar replace
>  field may contain any of the following sequences:
> @@ -2051,13 +2071,13 @@ The text matched by
>  .Ar pattern .
>  .It Sq \(a~
>  The replacement part of the previous
> -.Cm substitute
> +.Cm s
>  command.
>  .It Sq %
>  If this is the entire
>  .Ar replace
>  pattern, the replacement part of the previous
> -.Cm substitute
> +.Cm s
>  command.
>  .It Sq \e#
>  Where
> @@ -2082,6 +2102,18 @@ to be converted to uppercase.
>  Causes the next character to be converted to uppercase.
>  .El
>  .Pp
> +The
> +.Ar options
> +field may contain any of the following characters:
> +.Bl -tag -width Ds
> +.It Sq c
> +Prompt for confirmation before each replacement is done.
> +.It Sq g
> +Replace all instances of
> +.Ar pattern
> +in a line, not just the first.
> +.El
> +.Pp
>  .It Xo
>  .Cm su Ns Op Cm spend Ns
>  .Op Cm !\&
> @@ -2291,7 +2323,9 @@ Remember the values of the
>  and
>  .Sq g
>  suffixes to the
> -.Cm substitute
> +.Cm s , &
> +and
> +.Cm ~
>  commands, instead of initializing them as unset for each new command.
>  .It Cm escapetime Bq 1
>  The tenths of a second
>