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.

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

Implement multiline delimiter catpures #837

Closed
wants to merge 3 commits into from

Conversation

mkatychev
Copy link
Contributor

Implement multiline delimiter catpures

Issue: #835

Description

Implemented missing features for @append_multiline_delimiter and @prepend_multiline_delimiter documented in the project README

Checklist

Checklist before merging:

  • CHANGELOG.md updated
  • README.md up-to-date


if self.multi_line_nodes.contains(&parent_id) {
log::debug!(
parent_id;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this does tracing style key/value logging hence the activation of the kv feature flag in log, happy to remove. Made debug logs a bit cleaner for me.

Comment on lines +214 to +216
","? @do_nothing
.
")"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's an idempotence check failure here that's likely clashing with the @append_spaced_softline after the comma:

; Append softlines, unless followed by comments.
(
[
","
";"
] @append_spaced_softline
.
[(block_comment) (line_comment)]* @do_nothing
)

Logically, the precedence should be to handle the append delimiter before handling the sofline.

///
/// A boolean indicating whether a given node has a a parent labelled as multi-line.
/// If the provided node has no parent, the function returns `false`.
fn in_multiline_context(&self, node: &Node) -> bool {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there seems to be a lot of variance between referring to multilines, would help to normalize for consistency:

  • multiline
  • multi-line
  • multi_line

@Xophmeister
Copy link
Member

Thank you for taking this on, but before you continue, please see my comment on #835.

In particular, the @{prepend,append}_multiline_delimiter captures were removed along time ago (nearly 2 years ago) and replaced by a different approach using predicates. If you look at #354 -- where this happened and the documentation is correct -- that may solve your problem, without having to reimplement anything.

@mkatychev
Copy link
Contributor Author

Closed due to being obsolete:
#354

@mkatychev mkatychev closed this Jan 17, 2025
@Xophmeister Xophmeister mentioned this pull request Jan 20, 2025
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

Successfully merging this pull request may close these issues.

2 participants