Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(11)

Issue 95076: man/man1/ssam.1 review comments

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 10 months ago by jdc
Modified:
14 years, 7 months ago
Reviewers:
rsc_swtch
Visibility:
Public.

Patch Set 1 #

Total comments: 11

Patch Set 2 : incorporate Russ' suggestions #

Total comments: 2

Patch Set 3 : clean up options and unneeded code #

Total comments: 10

Patch Set 4 : style, first option command #

Patch Set 5 : handle options? really? never would have guessed #

Total comments: 12

Patch Set 6 : TMPDIR, no wq #

Patch Set 7 : add start range ',' if one command w/o -e #

Patch Set 8 : cosmetic code formatting #

Total comments: 8

Patch Set 9 : first draft from sed.1 #

Total comments: 15

Patch Set 10 : bin/vary - apply sam script to create a new file #

Patch Set 11 : review comments #

Total comments: 1

Patch Set 12 : bin/ssam: default range with k #

Total comments: 1

Patch Set 13 : word freq example; many-echo style #

Total comments: 1

Patch Set 14 : one-word-per-line example uses y and c #

Patch Set 15 : trivial \n to \en in man page source #

Total comments: 8

Patch Set 16 : start selection, *.ms, word freq #

Total comments: 2

Patch Set 17 : describe word freq example #

Patch Set 18 : provide both ssam and man page to codereview #

Unified diffs Side-by-side diffs Delta from patch set Stats (+115 lines, -0 lines) Patch
A bin/ssam View 1 2 3 4 5 6 7 8 12 1 chunk +43 lines, -0 lines 0 comments Download
A man/man1/ssam.1 View 9 11 13 14 15 16 1 chunk +72 lines, -0 lines 0 comments Download

Messages

Total messages: 49
jdc
14 years, 10 months ago (2009-07-18 23:49:23 UTC) #1
russcox_gmail.com
http://codereview.appspot.com/95076/diff/1/2 File bin/ssam (right): http://codereview.appspot.com/95076/diff/1/2#newcode4 Line 4: me=ssam replace $me throughout with ssam. http://codereview.appspot.com/95076/diff/1/2#newcode9 Line ...
14 years, 9 months ago (2009-07-25 19:22:01 UTC) #2
jdc
Rewrote most actual functionality to incorporate Russ' suggestions. Will upload new version. http://codereview.appspot.com/95076/diff/1/2 File bin/ssam ...
14 years, 9 months ago (2009-07-30 03:33:44 UTC) #3
jdc
14 years, 9 months ago (2009-07-30 03:36:12 UTC) #4
jdc
14 years, 9 months ago (2009-07-30 03:38:32 UTC) #5
russcox_gmail.com
http://codereview.appspot.com/95076/diff/3002/1003 File bin/ssam (right): http://codereview.appspot.com/95076/diff/3002/1003#newcode29 Line 29: if(! ~ $#flagn 0) cat $tmp Why did ...
14 years, 9 months ago (2009-08-08 20:38:06 UTC) #6
russcox_gmail.com
i think you're pretty close to done here. i hope you haven't given up.
14 years, 9 months ago (2009-08-15 06:52:23 UTC) #7
jdc
Not at all! I've been thinking about what you suggested (especially -n), and writing another ...
14 years, 9 months ago (2009-08-15 14:51:45 UTC) #8
jdc
http://codereview.appspot.com/95076/diff/3002/1003 File bin/ssam (right): http://codereview.appspot.com/95076/diff/3002/1003#newcode29 Line 29: if(! ~ $#flagn 0) cat $tmp On 2009/08/08 ...
14 years, 9 months ago (2009-08-17 01:12:20 UTC) #9
jdc
On 2009/08/17 01:12:20, jdc wrote: > http://codereview.appspot.com/95076/diff/3002/1003 > File bin/ssam (right): > > http://codereview.appspot.com/95076/diff/3002/1003#newcode29 > ...
14 years, 9 months ago (2009-08-17 01:18:48 UTC) #10
jdc
Updated ssam with your suggestions. I use the first-argument-as-command feature of sed all the time, ...
14 years, 9 months ago (2009-08-18 02:29:06 UTC) #11
jdc
14 years, 9 months ago (2009-08-18 02:45:33 UTC) #12
jdc
14 years, 9 months ago (2009-08-18 04:17:30 UTC) #13
russcox_gmail.com
looks great. let's move on to the man page. http://codereview.appspot.com/95076/diff/5004/5005 File bin/ssam (right): http://codereview.appspot.com/95076/diff/5004/5005#newcode3 Line ...
14 years, 9 months ago (2009-08-18 05:48:15 UTC) #14
russcox_gmail.com
http://codereview.appspot.com/95076/diff/5004/5005 File bin/ssam (right): http://codereview.appspot.com/95076/diff/5004/5005#newcode25 Line 25: { i wonder if this block should start ...
14 years, 9 months ago (2009-08-18 06:32:37 UTC) #15
jdc
Did some changes, but have issues with two of them, and an idea for another. ...
14 years, 9 months ago (2009-08-18 14:43:49 UTC) #16
jdc
14 years, 9 months ago (2009-08-18 14:44:50 UTC) #17
jdc
14 years, 8 months ago (2009-09-08 03:48:12 UTC) #18
jdc
14 years, 8 months ago (2009-09-08 03:51:01 UTC) #19
jdc
14 years, 8 months ago (2009-09-08 03:52:57 UTC) #20
jdc
14 years, 8 months ago (2009-09-10 04:46:00 UTC) #21
jdc
description of vary: Apply a sam script (a file with the suffix ".sam") to a ...
14 years, 8 months ago (2009-09-10 04:48:43 UTC) #22
rsc_swtch
let's leave vary out for now; it can go into a different issue #
14 years, 8 months ago (2009-09-10 05:09:38 UTC) #23
rsc_swtch
http://codereview.appspot.com/95076/diff/9004/8006 File man/man1/ssam.1 (right): http://codereview.appspot.com/95076/diff/9004/8006#newcode40 Line 40: will prepend ',' to the command. will prefix ...
14 years, 8 months ago (2009-09-10 05:15:42 UTC) #24
rsc_swtch
Let's make these changes to ssam and the manual and then get that added. vary ...
14 years, 8 months ago (2009-09-10 05:27:17 UTC) #25
jdc
http://codereview.appspot.com/95076/diff/9004/8006 File man/man1/ssam.1 (right): http://codereview.appspot.com/95076/diff/9004/8006#newcode40 Line 40: will prepend ',' to the command. On 2009/09/10 ...
14 years, 8 months ago (2009-09-10 18:53:13 UTC) #26
jdc
14 years, 8 months ago (2009-09-10 19:01:03 UTC) #27
russcox_gmail.com
http://codereview.appspot.com/95076/diff/9004/8006 File man/man1/ssam.1 (right): http://codereview.appspot.com/95076/diff/9004/8006#newcode48 Line 48: .B ssam -n '10p' file There's a difference ...
14 years, 8 months ago (2009-09-10 23:38:19 UTC) #28
jdc
I guess the effect of the 'k' lines is to select the entire range, for ...
14 years, 8 months ago (2009-09-12 23:56:31 UTC) #29
jdc
14 years, 8 months ago (2009-09-12 23:59:33 UTC) #30
rsc_swtch
When you reupload, please be sure to upload both ssam and ssam.1 http://codereview.appspot.com/95076/diff/9013/9014 File bin/ssam ...
14 years, 8 months ago (2009-09-13 21:43:23 UTC) #31
jdc
14 years, 8 months ago (2009-09-14 03:13:14 UTC) #32
rsc_swtch
http://codereview.appspot.com/95076/diff/10004/9018 File man/man1/ssam.1 (right): http://codereview.appspot.com/95076/diff/10004/9018#newcode49 Line 49: .B ssam 's,[^A-Za-z],\n,g' file Nice example. Let's use ...
14 years, 8 months ago (2009-09-14 04:19:55 UTC) #33
jdc
14 years, 8 months ago (2009-09-15 03:42:15 UTC) #34
jdc
14 years, 8 months ago (2009-09-15 03:46:18 UTC) #35
rsc_swtch
please fix these and upload ssam with it and then we can apply this. http://codereview.appspot.com/95076/diff/10008/9029 ...
14 years, 8 months ago (2009-09-15 16:30:59 UTC) #36
jdc
http://codereview.appspot.com/95076/diff/10008/9029 File man/man1/ssam.1 (right): http://codereview.appspot.com/95076/diff/10008/9029#newcode28 Line 28: begins with the entire file selected for the ...
14 years, 8 months ago (2009-09-15 19:51:28 UTC) #37
jdc
14 years, 8 months ago (2009-09-15 19:52:56 UTC) #38
rsc_swtch
Are you giving codereview bin/ssam too? It isn't showing up on the review screen from ...
14 years, 8 months ago (2009-09-15 20:13:27 UTC) #39
jdc
14 years, 8 months ago (2009-09-15 20:39:47 UTC) #40
rsc_swtch
this looks fine but ssam is still missing. you didn't answer my question: are you ...
14 years, 8 months ago (2009-09-15 21:05:45 UTC) #41
jdc
14 years, 8 months ago (2009-09-15 21:09:45 UTC) #42
jdc
Uploaded both files. I hadn't uploaded bin/ssam for a few since there wasn't a change. ...
14 years, 8 months ago (2009-09-15 21:12:05 UTC) #43
rsc_swtch
vary is gone by virtue of your not including it in the most recent patch ...
14 years, 8 months ago (2009-09-15 23:08:14 UTC) #44
old.codebot
# codebot: apply codebot error: unknown full name for jdc <jason.catena@gmail.com> jdc, please add a ...
14 years, 8 months ago (2009-09-15 23:08:30 UTC) #45
rsc_swtch
codebot: apply ssam(1): new command
14 years, 8 months ago (2009-09-15 23:20:33 UTC) #46
old.codebot
# codebot: apply applied.
14 years, 8 months ago (2009-09-15 23:22:10 UTC) #47
jdc
Thanks! Should I see both files on the commit page? Only the man page seems ...
14 years, 8 months ago (2009-09-15 23:27:33 UTC) #48
rsc_swtch
14 years, 7 months ago (2009-09-30 18:03:22 UTC) #49
On 2009/09/15 23:27:33, jdc wrote:
> Thanks!  Should I see both files on the commit page?  Only the man page seems
> there.

whoops; fixed
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b