Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

impl(generator): better blockquote handling #346

Merged
merged 1 commit into from
Dec 2, 2024
Merged

impl(generator): better blockquote handling #346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 59 additions & 11 deletions generator/internal/language/rust.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,21 +609,69 @@ func (*RustCodec) ToCamel(symbol string) string {
return rustEscapeKeyword(strcase.ToLowerCamel(symbol))
}

// TODO(#92) - protect all quotes with `norust`
// TODO(#30) - convert protobuf links to Rusty links.
//
// Blockquotes require special treatment for Rust. By default, Rust assumes
// blockquotes contain compilable Rust code samples. To override the default
// the blockquote must be marked with "```norust". The proto comments have
// many blockquotes that do not follow this convention (nor should they).
//
// This function handles some easy cases of blockquotes, but a full treatment
// requires parsing of the comments. The CommonMark [spec] includes some
// difficult examples.
//
// [spec]: https://spec.commonmark.org/0.13/#block-quotes
func (*RustCodec) FormatDocComments(documentation string) []string {
inBlockQuote := false
ss := strings.Split(documentation, "\n")
for i := range ss {
if strings.HasSuffix(ss[i], "```") {
if !inBlockQuote {
ss[i] = ss[i] + "norust"
inBlockquote := false
blockquotePrefix := ""

var results []string
for _, line := range strings.Split(documentation, "\n") {
if inBlockquote {
switch {
case line == "```":
inBlockquote = false
results = append(results, "```")
case strings.HasPrefix(line, blockquotePrefix):
results = append(results, strings.TrimPrefix(line, blockquotePrefix))
default:
inBlockquote = false
results = append(results, "```")
results = append(results, line)
}
} else {
switch {
case line == "```":
results = append(results, "```norust")
inBlockquote = true
case strings.HasPrefix(line, " "):
inBlockquote = true
blockquotePrefix = " "
results = append(results, "```norust")
results = append(results, strings.TrimPrefix(line, blockquotePrefix))
case strings.HasPrefix(line, " > "):
inBlockquote = true
blockquotePrefix = " > "
results = append(results, "```norust")
results = append(results, strings.TrimPrefix(line, blockquotePrefix))
case strings.HasPrefix(line, "> "):
inBlockquote = true
blockquotePrefix = "> "
results = append(results, "```norust")
results = append(results, strings.TrimPrefix(line, blockquotePrefix))
default:
results = append(results, line)
}
inBlockQuote = !inBlockQuote
}
ss[i] = fmt.Sprintf("/// %s", ss[i])
// nit: remove the trailing whitespace, this is unsightly.
ss[i] = strings.TrimRightFunc(ss[i], unicode.IsSpace)
}
return ss
if inBlockquote {
results = append(results, "```")
}
for i, line := range results {
results[i] = strings.TrimRightFunc(fmt.Sprintf("/// %s", line), unicode.IsSpace)
}
return results
}

func (c *RustCodec) projectRoot() string {
Expand Down
102 changes: 102 additions & 0 deletions generator/internal/language/rust_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,108 @@ func TestRust_FormatDocCommentsBullets(t *testing.T) {
}
}

func TestRust_FormatDocCommentsImplicitBlockQuote(t *testing.T) {
input := `
Blockquotes come in many forms. They can start with a leading '> ', as in:

> Block quote style 1
> Continues 1 - style 1
> Continues 2 - style 1
> Continues 3 - style 1

They can start with 3 spaces and then '> ', as in:

> Block quote style 2
> Continues 1 - style 2
> Continues 2 - style 2
> Continues 3 - style 2

Or they can start with just 4 spaces:

Block quote style 3
Continues 1 - style 3
Continues 2 - style 3
Continues 3 - style 3

Note that four spaces and a leading '> ' makes the '> ' prefix part of the
block:

> Block quote with arrow.
> Continues 1 - with arrow
> Continues 2 - with arrow
Continues 3 - with arrow

`

want := []string{
"///",
"/// Blockquotes come in many forms. They can start with a leading '> ', as in:",
"///",
"/// ```norust",
"/// Block quote style 1",
"/// Continues 1 - style 1",
"/// Continues 2 - style 1",
"/// Continues 3 - style 1",
"/// ```",
"///",
"/// They can start with 3 spaces and then '> ', as in:",
"///",
"/// ```norust",
"/// Block quote style 2",
"/// Continues 1 - style 2",
"/// Continues 2 - style 2",
"/// Continues 3 - style 2",
"/// ```",
"///",
"/// Or they can start with just 4 spaces:",
"///",
"/// ```norust",
"/// Block quote style 3",
"/// Continues 1 - style 3",
"/// Continues 2 - style 3",
"/// Continues 3 - style 3",
"/// ```",
"///",
"/// Note that four spaces and a leading '> ' makes the '> ' prefix part of the",
"/// block:",
"///",
"/// ```norust",
"/// > Block quote with arrow.",
"/// > Continues 1 - with arrow",
"/// > Continues 2 - with arrow",
"/// Continues 3 - with arrow",
"/// ```",
"///",
"///",
}

c := &RustCodec{}
got := c.FormatDocComments(input)
if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("mismatch in FormatDocComments (-want, +got)\n:%s", diff)
}
}

func TestRust_FormatDocCommentsImplicitBlockQuoteClosing(t *testing.T) {
input := `Blockquotes can appear at the end of the comment:

they should have a closing element.`

want := []string{
"/// Blockquotes can appear at the end of the comment:",
"///",
"/// ```norust",
"/// they should have a closing element.",
"/// ```",
}

c := &RustCodec{}
got := c.FormatDocComments(input)
if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("mismatch in FormatDocComments (-want, +got)\n:%s", diff)
}
}

func TestRust_MessageNames(t *testing.T) {
message := &api.Message{
Name: "Replication",
Expand Down
76 changes: 40 additions & 36 deletions generator/testdata/rust/gclient/golden/iam/v1/src/model.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,41 +437,43 @@ impl Binding {
///
/// Example Policy with multiple AuditConfigs:
///
/// ```norust
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These diffs are much easier to grok if you turn off whitespace differences.

/// {
/// "audit_configs": [
/// {
/// "audit_configs": [
/// "service": "allServices",
/// "audit_log_configs": [
/// {
/// "service": "allServices",
/// "audit_log_configs": [
/// {
/// "log_type": "DATA_READ",
/// "exempted_members": [
/// "user:[email protected]"
/// ]
/// },
/// {
/// "log_type": "DATA_WRITE"
/// },
/// {
/// "log_type": "ADMIN_READ"
/// }
/// "log_type": "DATA_READ",
/// "exempted_members": [
/// "user:[email protected]"
/// ]
/// },
/// {
/// "service": "sampleservice.googleapis.com",
/// "audit_log_configs": [
/// {
/// "log_type": "DATA_READ"
/// },
/// {
/// "log_type": "DATA_WRITE",
/// "exempted_members": [
/// "user:[email protected]"
/// ]
/// }
/// "log_type": "DATA_WRITE"
/// },
/// {
/// "log_type": "ADMIN_READ"
/// }
/// ]
/// },
/// {
/// "service": "sampleservice.googleapis.com",
/// "audit_log_configs": [
/// {
/// "log_type": "DATA_READ"
/// },
/// {
/// "log_type": "DATA_WRITE",
/// "exempted_members": [
/// "user:[email protected]"
/// ]
/// }
/// ]
/// }
/// ]
/// }
/// ```
///
/// For sampleservice, this policy enables DATA_READ, DATA_WRITE and ADMIN_READ
/// logging. It also exempts `[email protected]` from DATA_READ logging, and
Expand Down Expand Up @@ -510,19 +512,21 @@ impl AuditConfig {
/// Provides the configuration for logging a type of permissions.
/// Example:
///
/// ```norust
/// {
/// "audit_log_configs": [
/// {
/// "audit_log_configs": [
/// {
/// "log_type": "DATA_READ",
/// "exempted_members": [
/// "user:[email protected]"
/// ]
/// },
/// {
/// "log_type": "DATA_WRITE"
/// }
/// "log_type": "DATA_READ",
/// "exempted_members": [
/// "user:[email protected]"
/// ]
/// },
/// {
/// "log_type": "DATA_WRITE"
/// }
/// ]
/// }
/// ```
///
/// This enables 'DATA_READ' and 'DATA_WRITE' logging, while exempting
/// [email protected] from DATA_READ logging.
Expand Down
4 changes: 3 additions & 1 deletion generator/testdata/rust/gclient/golden/location/src/model.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ pub struct Location {

/// Cross-service attributes for the location. For example
///
/// {"cloud.googleapis.com/region": "us-east1"}
/// ```norust
/// {"cloud.googleapis.com/region": "us-east1"}
/// ```
#[serde(skip_serializing_if = "std::collections::HashMap::is_empty")]
pub labels: std::collections::HashMap<String, String>,

Expand Down
62 changes: 36 additions & 26 deletions generator/testdata/rust/gclient/golden/module/rpc/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,29 @@
/// Example of an error when contacting the "pubsub.googleapis.com" API when it
/// is not enabled:
///
/// { "reason": "API_DISABLED"
/// "domain": "googleapis.com"
/// "metadata": {
/// "resource": "projects/123",
/// "service": "pubsub.googleapis.com"
/// }
/// }
/// ```norust
/// { "reason": "API_DISABLED"
/// "domain": "googleapis.com"
/// "metadata": {
/// "resource": "projects/123",
/// "service": "pubsub.googleapis.com"
/// }
/// }
/// ```
///
/// This response indicates that the pubsub.googleapis.com API is not enabled.
///
/// Example of an error that is returned when attempting to create a Spanner
/// instance in a region that is out of stock:
///
/// { "reason": "STOCKOUT"
/// "domain": "spanner.googleapis.com",
/// "metadata": {
/// "availableRegions": "us-central1,us-east2"
/// }
/// }
/// ```norust
/// { "reason": "STOCKOUT"
/// "domain": "spanner.googleapis.com",
/// "metadata": {
/// "availableRegions": "us-central1,us-east2"
/// }
/// }
/// ```
#[serde_with::serde_as]
#[derive(Clone, Debug, Default, PartialEq, serde::Deserialize, serde::Serialize)]
#[serde(rename_all = "camelCase")]
Expand Down Expand Up @@ -336,21 +340,27 @@ pub mod bad_request {
///
/// Consider the following:
///
/// message CreateContactRequest {
/// message EmailAddress {
/// enum Type {
/// TYPE_UNSPECIFIED = 0;
/// HOME = 1;
/// WORK = 2;
/// }
/// ```norust
/// message CreateContactRequest {
/// message EmailAddress {
/// enum Type {
/// TYPE_UNSPECIFIED = 0;
/// HOME = 1;
/// WORK = 2;
/// }
/// ```
///
/// optional string email = 1;
/// repeated EmailType type = 2;
/// }
/// ```norust
/// optional string email = 1;
/// repeated EmailType type = 2;
/// }
/// ```
///
/// string full_name = 1;
/// repeated EmailAddress email_addresses = 2;
/// }
/// ```norust
/// string full_name = 1;
/// repeated EmailAddress email_addresses = 2;
/// }
/// ```
///
/// In this example, in proto `field` could take one of the following values:
///
Expand Down
Loading
Loading