Add docs for SubstitutionString #27134
Conversation
| """ | ||
| @s_str -> SubstitutionString | ||
|
|
||
| Construct a substitution string, used for regular expression substitutions. Within the |
There was a problem hiding this comment.
Need to escape all the backslashes?
There was a problem hiding this comment.
I've corrected that and fixed some doc reference errors (thanks to help from @fredrikekre), committed that to the PR.
There was a problem hiding this comment.
Actually you need single backslashes; the examples don't work with double ones. That's because non-standard string literal macros are passed the raw string, and escapes are not interpreted.
There was a problem hiding this comment.
The example is inside a julia string and escapes are interpreted.
There was a problem hiding this comment.
My bad, I chose the wrong thread. My comment applies to uses of s"..." below.
There was a problem hiding this comment.
Where? Everything here is inside a normal julia string.
There was a problem hiding this comment.
This example no longer works due to double slashes: https://github.com/JuliaLang/julia/pull/27134/files#diff-7422507bb03a8ecee35360dc9380b9caR261
There was a problem hiding this comment.
But it won't have double slashes in the rendered documentation? Again, because this is inside a normal julia string.
There was a problem hiding this comment.
Ahah, got it, that's tricky. Indeed that's a special string inside a standard (doc)string...
base/strings/util.jl
Outdated
| @@ -466,6 +466,9 @@ julia> replace("The quick foxes run quickly.", "quick" => "slow", count=1) | |||
|
|
|||
| julia> replace("The quick foxes run quickly.", "quick" => "", count=1) | |||
| "The foxes run quickly." | |||
|
|
|||
| julia> replace("The quick foxes run quickly.", r"fox(es)?" => s"bus\1") | |||
digital-carver
force-pushed
the
subst_str_doc
branch
from
2bfb802 to
e56d839
Compare
base/regex.jl
Outdated
| SubstitutionString(substr) | ||
|
|
||
| Stores the given string `substr` as a SubstitutionString, for use in regular expression | ||
| substitutions. Most commonly constructed using the [`@s_str`](@ref) macro. |
There was a problem hiding this comment.
Would be nice to show an example of how to use the macro, as the link between @s_str and s"..." isn't completely obvious.
There was a problem hiding this comment.
As I mentioned in the other comment, I'm leaning towards just merging this into the doc for @s_str and removing the doc here. That would also parallel what's done with Regex - @r_str has documentation, the type itself doesn't. The link in replace's doc will then just have to be changed to "r is a SubstitutionString (constructed with @s_str)", which would lead the user more directly to the actual construct they'd need to use in the code. Do you think that make sense?
There was a problem hiding this comment.
I'd rather keep this docstring, especially since you've written it. ;-) The fact that Regex isn't documented is rather something to fix: all public types and methods should have docstrings.
There was a problem hiding this comment.
I've added an example of using the s"..." macro and added SubstitutionString to exports.jl. Does this PR need anything else or is this good to go?
| """ | ||
| @s_str -> SubstitutionString | ||
|
|
||
| Construct a substitution string, used for regular expression substitutions. Within the |
There was a problem hiding this comment.
Actually you need single backslashes; the examples don't work with double ones. That's because non-standard string literal macros are passed the raw string, and escapes are not interpreted.
| @@ -21,6 +21,8 @@ Base.codeunit | |||
| Base.codeunits | |||
| Base.ascii | |||
| Base.@r_str | |||
| Base.SubstitutionString | |||
There was a problem hiding this comment.
AFAIK we only list exported objects here, yet SubstitutionString isn't exported. I guess it should be?
There was a problem hiding this comment.
There seem to be a few exceptions to that (eg. BroadcastStyle in arrays.md, OneTo in math.md). I added SubstitutionString here only because Documenter needed that to be able to generate cross-reference from replace's doc. I'm not sure it makes sense to export SubstitutionString though: it's somewhat cumbersome to use as a constructor (with the longer name than s"" and the need for additional backslashes), and as a type it's useful (afaict) only to the very few people building RegEx libraries. Perhaps I should go with my original idea of only documenting @s_str and linking directly to that from replace's doc - that's probably more useful from a practical point of view too.
There was a problem hiding this comment.
Regex is exported even though the same objections apply, so I don't think that's a problem.
digital-carver
force-pushed
the
subst_str_doc
branch
from
e56d839 to
dedca7d
Compare
kshyatt
added
docs
|
@StefanKarpinski Is it OK to export |
|
Sure, go for it. |
|
Thanks @digital-carver! |
Resolves #26497