Skip to content

Update sed documentation regarding capture groups#558

Merged
freitagbr merged 3 commits intoshelljs:masterfrom
freitagbr:sed-documentation
Nov 18, 2016
Merged

Update sed documentation regarding capture groups#558
freitagbr merged 3 commits intoshelljs:masterfrom
freitagbr:sed-documentation

Conversation

@freitagbr
Copy link
Copy Markdown
Contributor

Fixes #507

Copy link
Copy Markdown
Member

@nfischer nfischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See inline comments

src/sed.js Outdated
//@
//@ Note:
//@
//@ Unix `sed` specifies capture groups using `\n` syntax. In ShellJS (and JavaScript, in general),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use \n, since that sounds like the newline character. It might suffice to just provide the javascript example and skip the shell code, for brevity's sake.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, it would probably be best to just mention sed's capture groups by name, and then show the JS example.

@nfischer
Copy link
Copy Markdown
Member

@freitagbr LGTM once you regenerate docs

@nfischer nfischer added the docs label Nov 17, 2016
@nfischer nfischer added this to the v0.7.x milestone Nov 17, 2016
@nfischer
Copy link
Copy Markdown
Member

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants