Add documentation for Expressions #76

jwhb · 2024-10-25T21:43:13Z

This is a major improvement to the expr module:

Adds doc comments throughout the module
Adds Default impls for all structs in expr
(breaking) Adds SocketAttr enum type for Socket key.

Adds impls of Default for several expr types. Adds doc comments throughout expr module. BREAKING CHANGE: Adds SocketAttr enum type for Socket key.

JKRhb

Apart from a few nits: LGTM! :)

JKRhb · 2025-01-09T21:50:16Z

src/expr.rs

    Verdict(Verdict<'a>),
 }

 #[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "lowercase")]
-/// Wrapper for non-immediate `Expression`s.
+/// Wrapper for non-immediate [Expressions](Expression).


Not sure if this should be lowercase, for the sake of consistency?

Suggested change

/// Wrapper for non-immediate [Expressions](Expression).

/// Wrapper for non-immediate [expressions](Expression).

Yep. Also I noticed that rustdoc triggers a warning for redundant labels (e.g. [Flag](Flag))

JKRhb · 2025-01-09T21:51:51Z

src/expr.rs

    Fib(Fib),
+    /// Explicitly set element object, in case `timeout`, `expires` or `comment`


Suggested change

/// Explicitly set element object, in case `timeout`, `expires` or `comment`

/// Explicitly set element object, in case `timeout`, `expires`, or `comment`

JKRhb · 2025-01-09T21:52:28Z

src/expr.rs

    Elem(Elem<'a>),
+    /// Construct a reference to packet’s socket.


Suggested change

/// Construct a reference to packet’s socket.

/// Construct a reference to a packet’s socket.

JKRhb · 2025-01-09T21:53:24Z

src/expr.rs

    pub addr: Box<Expression<'a>>,
+    /// A prefix length.


Suggested change

/// A prefix length.

/// The prefix length.

JKRhb · 2025-01-09T21:55:10Z

src/expr.rs

    Socket(Socket<'a>),
+    /// Perform OS fingerprinting.
+    /// This expression is typically used in the LHS of a [match](crate::stmt::Match) statement.


I guess the second sentence/explanation should be separated by an empty line.

Suggested change

/// This expression is typically used in the LHS of a [match](crate::stmt::Match) statement.

///

/// This expression is typically used in the LHS of a [match](crate::stmt::Match) statement.

JKRhb · 2025-01-09T22:01:39Z

src/expr.rs

 #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, Serialize, Deserialize)]
 #[serde(rename_all = "lowercase")]
 /// Represents a `meta` key for packet meta data.
+///
+/// See also: <https://wiki.nftables.org/wiki-nftables/index.php/Matching_packet_metainformation>


Suggested change

/// See also: <https://wiki.nftables.org/wiki-nftables/index.php/Matching_packet_metainformation>

/// See [this page](https://wiki.nftables.org/wiki-nftables/index.php/Matching_packet_metainformation)

for more information.

JKRhb · 2025-01-09T22:03:13Z

src/expr.rs

+    Oifkind,
+    /// Output interface group.
+    Oifgroup,
+    /// Input bridge interface name.
    Ibridgename,


Hmm, should these rather be sorted alphabetically?

JKRhb · 2025-01-09T22:17:24Z

src/lib.rs

@@ -11,7 +11,7 @@
 /// Contains Batch object to be used to prepare Nftables payloads.
 pub mod batch;

-/// Contains Expressions.
+/// Contains [Expressions](crate::expr::Expression).


Suggested change

/// Contains [Expressions](crate::expr::Expression).

/// Contains [expressions](crate::expr::Expression).

JKRhb · 2025-01-09T22:18:42Z

src/expr.rs

+    Transparent,
+    /// Match on the socket mark (`SOL_SOCKET`, `SO_MARK`).
+    Mark,
+    /// Indicates whether the socket is wildcard-bound (e.g. 0.0.0.0 or ::0).


I think :: does not need a 0 suffix, right?

Suggested change

/// Indicates whether the socket is wildcard-bound (e.g. 0.0.0.0 or ::0).

/// Indicates whether the socket is wildcard-bound (e.g. 0.0.0.0 or ::).

JKRhb · 2025-01-09T22:19:51Z

src/expr.rs

+    /// Similar to jump, but the current position is not pushed to the call
+    /// stack, meaning that after the new chain evaluation will continue at the
+    /// last chain instead of the one containing the goto statement.


Suggested change

/// Similar to jump, but the current position is not pushed to the call

/// stack, meaning that after the new chain evaluation will continue at the

/// last chain instead of the one containing the goto statement.

/// Similar to jump, but the current position is not pushed to the call

/// stack.

///

/// That means that after the new chain evaluation will continue at the

/// last chain instead of the one containing the goto statement.

jwhb force-pushed the jwhb-expr-docs branch from 5003a46 to f7e93a0 Compare October 25, 2024 21:44

jwhb self-assigned this Oct 25, 2024

jwhb added documentation Improvements or additions to documentation enhancement New feature or request labels Oct 25, 2024

jwhb added this to the v0.6.0 milestone Oct 25, 2024

jwhb linked an issue Oct 25, 2024 that may be closed by this pull request

Implement remaining statements (last, map, reset, secmark) #70

Open

jwhb force-pushed the jwhb-expr-docs branch from f7e93a0 to 900734c Compare November 8, 2024 10:40

jwhb force-pushed the main branch from ea86eca to 021668a Compare January 7, 2025 13:19

jwhb force-pushed the jwhb-expr-docs branch 4 times, most recently from 36f2faa to a4fdf39 Compare January 9, 2025 20:30

feat(expr)!: add expr default impl, improve socket

b7ae445

Adds impls of Default for several expr types. Adds doc comments throughout expr module. BREAKING CHANGE: Adds SocketAttr enum type for Socket key.

jwhb force-pushed the jwhb-expr-docs branch from a4fdf39 to b7ae445 Compare January 9, 2025 20:36

jwhb marked this pull request as ready for review January 9, 2025 20:40

jwhb requested a review from JKRhb January 9, 2025 20:40

jwhb removed a link to an issue Jan 9, 2025

Implement remaining statements (last, map, reset, secmark) #70

Open

JKRhb approved these changes Jan 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add documentation for Expressions #76

Add documentation for Expressions #76

jwhb commented Oct 25, 2024 •

edited

Loading

JKRhb left a comment

JKRhb Jan 9, 2025

jwhb Jan 9, 2025 •

edited

Loading

JKRhb Jan 9, 2025

JKRhb Jan 9, 2025

JKRhb Jan 9, 2025

JKRhb Jan 9, 2025

JKRhb Jan 9, 2025

JKRhb Jan 9, 2025

JKRhb Jan 9, 2025

JKRhb Jan 9, 2025

JKRhb Jan 9, 2025

	/// Wrapper for non-immediate [Expressions](Expression).
	/// Wrapper for non-immediate [expressions](Expression).

		Fib(Fib),
		/// Explicitly set element object, in case `timeout`, `expires` or `comment`

		Elem(Elem<'a>),
		/// Construct a reference to packet’s socket.

	/// Construct a reference to packet’s socket.
	/// Construct a reference to a packet’s socket.

	/// This expression is typically used in the LHS of a [match](crate::stmt::Match) statement.
	///
	/// This expression is typically used in the LHS of a [match](crate::stmt::Match) statement.

	/// See also: <https://wiki.nftables.org/wiki-nftables/index.php/Matching_packet_metainformation>
	/// See [this page](https://wiki.nftables.org/wiki-nftables/index.php/Matching_packet_metainformation)
	for more information.

	/// Contains [Expressions](crate::expr::Expression).
	/// Contains [expressions](crate::expr::Expression).

	/// Indicates whether the socket is wildcard-bound (e.g. 0.0.0.0 or ::0).
	/// Indicates whether the socket is wildcard-bound (e.g. 0.0.0.0 or ::).

Add documentation for Expressions #76

Are you sure you want to change the base?

Add documentation for Expressions #76

Conversation

jwhb commented Oct 25, 2024 • edited Loading

JKRhb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwhb Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwhb commented Oct 25, 2024 •

edited

Loading

jwhb Jan 9, 2025 •

edited

Loading