Skip to content

Feature: All trace attribute arguments are named/keyword arguments #142

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.

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

Closed
taqtiqa-mark opened this issue Apr 27, 2022 · 1 comment
Closed

Comments

@taqtiqa-mark
Copy link
Contributor

taqtiqa-mark commented Apr 27, 2022

Currently the #[trace] attribute is a mixture of:

  1. optional, required or prohibited arguments
  2. named/keyword and positional arguments

This makes the attribute logic brittle and more difficult to extend than it needs to be. Specifically, the trace name is a positional argument which is both required and optional:

  1. from the attribute logic PoV it is required
  2. from the user PoV it is optional

Other arguments can be prohibited or required from the attribute logic PoV, e.g. enter_on_poll is prohibited for non-async functions, but required for async functions.

We can't do anything about the required/prohibited/optional properties - these arise naturally from the domain.

We can simplify required/prohibited/optional parsing logic by not complicating things with positional and named/keyword parameter categories.
That is, by removing the use of a positional arguments.

Specifically, in the course of issue #113, landing as PR #127, the following supported syntax has simplified matters by eliminating the need for a validation stage, leaving only a parse stage:

  1. #[trace] fn f(){} shorthand for #[trace(name='f')] fn f(){}
  2. #[trace(enter_on_poll=true)] async fn f(){} shorthand for #[trace(enter_on_poll=true,name='f')] async fn f(){} . Note: Here order is immaterial.
  3. #[trace(name='a')] fn f(){}

The existing mixture of named and positional arguments would no longer be accepted:

  1. #[trace] fn f(){} shorthand for #[trace('f')] fn f(){}
  2. #[trace(enter_on_poll=true)] async fn f(){} shorthand for #[trace(f,enter_on_poll=true)] async fn f(){} . Note: Here order is material.
  3. #[trace('a')] fn f(){}

This would improve the readability of code.
This would deviate from the tracing crate convention, depending on your PoV this is a positive or negative.

Once there is a comprehensive test suite in place and all bugs are eliminated - perhaps then we could consider adding subtle corner cases to the parsing logic by supporting a mixture of positional and named/keyword arguments?

Thoughts?

@taqtiqa-mark taqtiqa-mark changed the title Feature: All trace attribute arguments are named arguments Feature: All trace attribute arguments are named/keyword arguments Apr 27, 2022
@andylokandy
Copy link
Collaborator

implemented in #153

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants