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

keep-sorted could handle trailing commas better #33

Open
JeffFaer opened this issue Apr 3, 2024 · 1 comment
Open

keep-sorted could handle trailing commas better #33

JeffFaer opened this issue Apr 3, 2024 · 1 comment

Comments

@JeffFaer
Copy link
Collaborator

JeffFaer commented Apr 3, 2024

There's some logic in keep-sorted to try and gracefully handle lists that don't have a final trailing comma like

ImmutableList.of(
  // keep-sorted start
  1,
  3,
  2
  // keep-sorted end
);

would become the following (commas added and removed as necessary)

ImmutableList.of(
  // keep-sorted start
  1,
  2,
  3
  // keep-sorted end
);

// handleTrailingComma handles the special case that all lines of a sorted segment are terminated
// by a comma except for the final element; in this case, we add a ',' to the
// last linegroup and strip it again after sorting.

There's a couple edge cases here that aren't handled very well right now

  1. Trailing comments after a comma

    This prevents the special case from being triggered

    ImmutableList.of(
      // keep-sorted start
      1,
      3, // three
      2
      // keep-sorted end
    );

    would become

    ImmutableList.of(
      // keep-sorted start
      1,
      2
      3, // three
      // keep-sorted end
    );
  2. Trailing comment after the last line

    The logic still triggers, but we add a comma to the comment

    ImmutableList.of(
      // keep-sorted start
      1,
      3, 
      2 // two
      // keep-sorted end
    );

    would become

    ImmutableList.of(
      // keep-sorted start
      1,
      2 // two,
      3
      // keep-sorted end
    );
@AlexanderGH
Copy link

Here's my take on both handling this and generalizing it to avoid hard-coding one-off logic concepts: AlexanderGH/presubmitchecks#7

It's not an exact 1:1 because it doesn't support stuff like this:

(it would maintain the missing comma in the same position) but if i wanted to, i could do a pre-sort by prefix order, but that seems like too much of a special-case and trying to again put language knowledge into the system.

tl;dr: accept a regexp with two subgroups. one subgroup is sticky to the sortable group, and the other is sticky to the position (anything outside the subgroups is dropped). In this example the comma would be sticky to the group position and the comment sticky to the sortable group.

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