Skip to content

Commit

Permalink
Provide max line-length in comment filter. (open-telemetry#454)
Browse files Browse the repository at this point in the history
* Merge with main.

* Fix clippy issues.

* Update go template to more accurrately match go style.  Update expected output to use go fmt.

* Add markdown link parser to word split algorithm

* Fix accidental split of words by hyphens

* Add dynamic line-length argument and test.

* Update docs for line_length config.

* Update HTML rendering to do word-wrapping on its own.

* Fix tests.

* Abstract out word-wrapping features of HTML renderer.

* Add word wrapping to markdown renderer.

* Get all unit tests passing.

* Fix up markdown word wrap to pass more tests.

* Fix issue with word-wrap tracking.  Two remaining go rendering issues.

* Clean up architecture of word wrapping.

* Add go list-style rendering for markdown format.

* Fix clippy.

* update go example to deal with newline hell in jinja.

* Fix spelling issue.

* Fix spelling.

* Update crates/weaver_forge/src/formats/markdown.rs

Co-authored-by: Laurent Quérel <[email protected]>

* Fixes from code review.

---------

Co-authored-by: Laurent Quérel <[email protected]>
Co-authored-by: Laurent Quérel <[email protected]>
  • Loading branch information
3 people authored Nov 24, 2024
1 parent 1529f30 commit aeae196
Show file tree
Hide file tree
Showing 13 changed files with 1,873 additions and 529 deletions.
574 changes: 309 additions & 265 deletions Cargo.lock

Large diffs are not rendered by default.

13 changes: 13 additions & 0 deletions crates/weaver_diff/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,19 @@ pub fn diff_dir<P: AsRef<Path>>(expected_dir: P, observed_dir: P) -> std::io::Re
Ok(are_identical)
}

#[macro_export]
/// Macro to simplify comparing two strings with a diff.
macro_rules! assert_string_eq {
($lhs:expr, $rhs:expr) => {
assert_eq!(
$lhs,
$rhs,
"Found string differences: {}",
weaver_diff::diff_output($lhs, $rhs)
);
};
}

/// Canonicalizes a JSON string by parsing it into a `serde_json::Value` and then canonicalizing it.
///
/// # Returns
Expand Down
1 change: 1 addition & 0 deletions crates/weaver_forge/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ jaq-syn = "1.1.0"
indexmap = "2.6.0"
regex = "1.11.1"
markdown = "=1.0.0-alpha.21"
textwrap = "0.16.1"

itertools.workspace = true
thiserror.workspace = true
Expand Down
5 changes: 5 additions & 0 deletions crates/weaver_forge/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -598,13 +598,17 @@ comment_formats: # optional
trim: <bool> # Flag to trim the comment content (default: true).
remove_trailing_dots: <bool> # Flag to remove trailing dots from the comment content (default: false).
enforce_trailing_dots: <bool> # Flag to enforce trailing dots for the comment content (default: false).
word_wrap:
line_length: <int> # Maximum number of characters on a line (default: unlimited).
ignore_newlines: <bool> # Whether newlines in comment should be ignored when a max line_length is set (default: false).

# Fields specific to 'markdown' format
escape_backslashes: <bool> # Whether to escape backslashes in markdown (default: false).
escape_square_brackets: <bool> # Whether to escape square brackets in markdown (default: false).
shortcut_reference_links: <bool> # Convert inlined links into shortcut reference links (default: false).
indent_first_level_list_items: <bool> # Indent the first level of list items in markdown (default: false).
default_block_code_language: <string> # Default language for block code snippets (default: "").
use_go_style_list_indent: <bool>, # Whether to use different indent spacing for ordered and unordered lists (default: false).

# Fields specific to 'html' format
old_style_paragraph: <bool> # Use old-style HTML paragraphs (default: false).
Expand Down Expand Up @@ -862,6 +866,7 @@ The `comment` filter accepts the following optional parameters:
- **`footer`**: A custom footer for the comment block.
- **`indent`**: Number of spaces to add before each comment line for indentation purposes.
- **`indent_type`**: The type of indentation to use. Supported values are `space` (default) and `tab`.
- **`line_length`**: The maximum number of characters in a line.

> Please open an issue if you have any suggestions for new formats or features.
Expand Down
233 changes: 126 additions & 107 deletions crates/weaver_forge/expected_output/comment_format/example.go
Original file line number Diff line number Diff line change
@@ -1,122 +1,141 @@
// Examples of comments for group device
package test

// DEVICE_ID
// A unique identifier representing the device
//
// The device identifier MUST only be defined using the values outlined below. This value is not an advertising identifier and MUST NOT be used as such. On iOS (Swift or Objective-C), this value MUST be equal to the [vendor identifier]. On Android (Java or Kotlin), this value MUST be equal to the Firebase Installation ID or a globally unique UUID which is persisted across sessions in your application. More information can be found [here] on best practices and exact implementation details. Caution should be taken when storing personal data or anything which can identify a user. GDPR and data protection laws may apply, ensure you do your own due diligence
//
// [vendor identifier]: https://developer.apple.com/documentation/uikit/uidevice/1620059-identifierforvendor
// [here]: https://developer.android.com/training/articles/user-data-ids
const DEVICE_ID = ""
// Examples of comments for group device

// DEVICE_MANUFACTURER
// The name of the device manufacturer
//
// The Android OS provides this field via [Build]. iOS apps SHOULD hardcode the value `Apple`
//
// [Build]: https://developer.android.com/reference/android/os/Build#MANUFACTURER
const DEVICE_MANUFACTURER = ""
// DEVICE_ID
// A unique identifier representing the device
//
// The device identifier MUST only be defined using the values outlined below.
// This value is not an advertising identifier and MUST NOT be used as such. On
// iOS (Swift or Objective-C), this value MUST be equal to the
// [vendor identifier]. On Android (Java or Kotlin), this value MUST be equal to
// the Firebase Installation ID or a globally unique UUID which is persisted
// across sessions in your application. More information can be found [here] on
// best practices and exact implementation details. Caution should be taken when
// storing personal data or anything which can identify a user. GDPR and data
// protection laws may apply, ensure you do your own due diligence
//
// [vendor identifier]: https://developer.apple.com/documentation/uikit/uidevice/1620059-identifierforvendor
// [here]: https://developer.android.com/training/articles/user-data-ids
const DEVICE_ID = ""

// DEVICE_MODEL_IDENTIFIER
// The model identifier for the device
//
// It's recommended this value represents a machine-readable version of the model identifier rather than the market or consumer-friendly name of the device
const DEVICE_MODEL_IDENTIFIER = ""
// DEVICE_MANUFACTURER
// The name of the device manufacturer
//
// The Android OS provides this field via [Build]. iOS apps SHOULD hardcode the
// value `Apple`
//
// [Build]: https://developer.android.com/reference/android/os/Build#MANUFACTURER
const DEVICE_MANUFACTURER = ""

// DEVICE_MODEL_NAME
// The marketing name for the device model
//
// It's recommended this value represents a human-readable version of the device model rather than a machine-readable alternative
const DEVICE_MODEL_NAME = ""
// DEVICE_MODEL_IDENTIFIER
// The model identifier for the device
//
// It's recommended this value represents a machine-readable version of the model
// identifier rather than the market or consumer-friendly name of the device
const DEVICE_MODEL_IDENTIFIER = ""

// DEVICE_MODEL_NAME
// The marketing name for the device model
//
// It's recommended this value represents a human-readable version of the device
// model rather than a machine-readable alternative
const DEVICE_MODEL_NAME = ""

// Examples of comments for group dns

// DNS_QUESTION_NAME
// The name being queried.
//
// If the name field contains non-printable characters (below 32 or above 126), those characters should be represented as escaped base 10 integers (\DDD). Back slashes and quotes should be escaped. Tabs, carriage returns, and line feeds should be converted to \t, \r, and \n respectively
const DNS_QUESTION_NAME = ""

// DNS_QUESTION_NAME
// The name being queried.
//
// If the name field contains non-printable characters (below 32 or above 126),
// those characters should be represented as escaped base 10 integers (\DDD).
// Back slashes and quotes should be escaped. Tabs, carriage returns, and line
// feeds should be converted to \t, \r, and \n respectively
const DNS_QUESTION_NAME = ""

// Examples of comments for group error

// ERROR_TYPE
// Describes a class of error the operation ended with.
//
// The `error.type` SHOULD be predictable, and SHOULD have low cardinality.
//
// When `error.type` is set to a type (e.g., an exception type), its
// canonical class name identifying the type within the artifact SHOULD be used.
//
// Instrumentations SHOULD document the list of errors they report.
//
// The cardinality of `error.type` within one instrumentation library SHOULD be low.
// Telemetry consumers that aggregate data from multiple instrumentation libraries and applications
// should be prepared for `error.type` to have high cardinality at query time when no
// additional filters are applied.
//
// If the operation has completed successfully, instrumentations SHOULD NOT set `error.type`.
//
// If a specific domain defines its own set of error identifiers (such as HTTP or gRPC status codes),
// it's RECOMMENDED to:
//
// - Use a domain-specific attribute
// - Set `error.type` to capture all errors, regardless of whether they are defined within the domain-specific set or not
const ERROR_TYPE = ""

// ERROR_TYPE
// Describes a class of error the operation ended with.
//
// The `error.type` SHOULD be predictable, and SHOULD have low cardinality.
//
// When `error.type` is set to a type (e.g., an exception type), its
// canonical class name identifying the type within the artifact SHOULD be used.
//
// Instrumentations SHOULD document the list of errors they report.
//
// The cardinality of `error.type` within one instrumentation library SHOULD be
// low.
// Telemetry consumers that aggregate data from multiple instrumentation
// libraries and applications
// should be prepared for `error.type` to have high cardinality at query time
// when no
// additional filters are applied.
//
// If the operation has completed successfully, instrumentations SHOULD NOT set
// `error.type`.
//
// If a specific domain defines its own set of error identifiers (such as HTTP or
// gRPC status codes),
// it's RECOMMENDED to:
//
// - Use a domain-specific attribute
// - Set `error.type` to capture all errors, regardless of whether they are
// defined within the domain-specific set or not
const ERROR_TYPE = ""

// Examples of comments for group other

// ATTR
// This is a brief description of the attribute + a short link [OTEL].
//
// This is a note about the attribute `attr`. It can be multiline.
//
// It can contain a list:
//
// - item **1**,
// - lorem ipsum dolor sit amet, consectetur
// adipiscing elit sed do eiusmod tempor
// [incididunt] ut labore et dolore magna aliqua.
// - item 2
// - lorem ipsum dolor sit amet, consectetur
// adipiscing elit sed do eiusmod tempor
// incididunt ut labore et dolore magna aliqua.
//
// And an **inline code snippet**: `Attr.attr`.
//
// # Summary
//
// ## Examples
//
// 1. Example 1
// 2. [Example] with lorem ipsum dolor sit amet, consectetur adipiscing elit
// [sed] do eiusmod tempor incididunt ut
// [labore] et dolore magna aliqua.
// 3. Example 3
//
// ## Appendix
//
// - [Link 1]
// - [Link 2]
// - A very long item in the list with lorem ipsum dolor sit amet, consectetur adipiscing elit sed do eiusmod
// tempor incididunt ut labore et dolore magna aliqua.
//
// > This is a blockquote.
// > It can contain multiple lines.
// > Lorem ipsum dolor sit amet, consectetur adipiscing
// > elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
//
// > [!NOTE] Something very important here
//
// [OTEL]: https://www.opentelemetry.com
// [incididunt]: https://www.loremipsum.com
// [Example]: https://loremipsum.com
// [sed]: https://loremipsum.com
// [labore]: https://loremipsum.com
// [Link 1]: https://www.link1.com
// [Link 2]: https://www.link2.com
const ATTR = ""

// ATTR
// This is a brief description of the attribute + a short link [OTEL].
//
// This is a note about the attribute `attr`. It can be multiline.
//
// It can contain a list:
//
// - item **1**,
// - lorem ipsum dolor sit amet, consectetur
// adipiscing elit sed do eiusmod tempor
// [incididunt] ut labore et dolore magna aliqua.
// - item 2
// - lorem ipsum dolor sit amet, consectetur
// adipiscing elit sed do eiusmod tempor
// incididunt ut labore et dolore magna aliqua.
//
// And an **inline code snippet**: `Attr.attr`.
//
// # Summary
//
// ## Examples
//
// 1. Example 1
// 2. [Example] with lorem ipsum dolor sit amet, consectetur adipiscing elit
// [sed] do eiusmod tempor incididunt ut
// [labore] et dolore magna aliqua.
// 3. Example 3
//
// ## Appendix
//
// - [Link 1]
// - [Link 2]
// - A very long item in the list with lorem ipsum dolor sit amet, consectetur
// adipiscing elit sed do eiusmod
// tempor incididunt ut labore et dolore magna aliqua.
//
// > This is a blockquote.
// > It can contain multiple lines.
// > Lorem ipsum dolor sit amet, consectetur adipiscing
// > elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
//
// > [!NOTE] Something very important here
//
// [OTEL]: https://www.opentelemetry.com
// [incididunt]: https://www.loremipsum.com
// [Example]: https://loremipsum.com
// [sed]: https://loremipsum.com
// [labore]: https://loremipsum.com
// [Link 1]: https://www.link1.com
// [Link 2]: https://www.link2.com
const ATTR = ""

5 changes: 5 additions & 0 deletions crates/weaver_forge/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use crate::error::Error::InvalidConfigFile;
use crate::file_loader::{FileContent, FileLoader};
use crate::formats::html::HtmlRenderOptions;
use crate::formats::markdown::MarkdownRenderOptions;
use crate::formats::WordWrapConfig;
use crate::WEAVER_YAML;

/// Weaver configuration.
Expand Down Expand Up @@ -322,6 +323,10 @@ pub struct CommentFormat {
/// Flag to enforce trailing dots on the comment content.
#[serde(default = "default_bool::<false>")]
pub enforce_trailing_dots: bool,

/// Configuration of word-wrapping behavior for comments.
#[serde(default)]
pub word_wrap: WordWrapConfig,
}

/// The type of indentation to use for the comment.
Expand Down
8 changes: 8 additions & 0 deletions crates/weaver_forge/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,14 @@ impl From<Error> for DiagnosticMessages {
}
}

impl From<std::fmt::Error> for Error {
fn from(_: std::fmt::Error) -> Self {
Self::TemplateEngineError {
error: "Unexpected string formatting error".to_owned(),
}
}
}

#[must_use]
pub(crate) fn jinja_err_convert(e: minijinja::Error) -> Error {
Error::WriteGeneratedCodeFailed {
Expand Down
Loading

0 comments on commit aeae196

Please sign in to comment.