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

make the tiles on a selection a struct of info, not twinned lists #143

Conversation

bmschmidt
Copy link
Collaborator

@bmschmidt bmschmidt commented Sep 16, 2024

Currently, selections maintain a list of tiles and a list of how many matches there are in them that are guaranteed to be the same length by code.

It's better to have a list of records where each record contains both of these facts; we'll be using these much more to store sort information in a bit.


🚀 This description was created by 927fc1c

refactor: use SelectionTile in DataSelection for tile management

Summary:

Refactor DataSelection to use SelectionTile for managing tiles and match counts, simplifying data handling and preparing for future enhancements.

Key points:

  • Refactor DataSelection to use SelectionTile for managing tiles and match counts.
  • Introduce SelectionTile class in src/selection.ts.
  • Replace separate tiles and match_count lists with a single list of SelectionTile objects.
  • Update methods: moveCursorToPoint(), add_or_remove_points(), wrapWithSelectionMetadata().
  • Adjust iteration and selection logic for SelectionTile.
  • Fix typo in comment: "untile" to "until".

Generated with ❤️ by ellipsis.dev

Copy link
Collaborator Author

bmschmidt commented Sep 16, 2024

This was referenced Sep 16, 2024
@bmschmidt bmschmidt assigned RLesser and eelegiap and unassigned RLesser Sep 16, 2024
@bmschmidt bmschmidt requested a review from eelegiap September 16, 2024 19:48
@bmschmidt bmschmidt marked this pull request as ready for review September 16, 2024 19:48
@bmschmidt bmschmidt force-pushed the 08-07-make_the_tiles_on_a_selection_a_struct_of_info_not_twinned_lists branch from b59865f to 6fda8c1 Compare September 19, 2024 19:00
Copy link
Collaborator Author

bmschmidt commented Sep 19, 2024

Merge activity

@bmschmidt bmschmidt force-pushed the 08-07-make_the_tiles_on_a_selection_a_struct_of_info_not_twinned_lists branch from 6fda8c1 to 3c779e2 Compare September 20, 2024 02:56
// The match count is the number of matches **per tile**;
// used to access numbers by index.
public matchCount?: number;
Copy link

Choose a reason for hiding this comment

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

The matchCount property in SelectionTile is optional, but it is always used as a number. It should be initialized to 0 in the constructor to avoid potential undefined errors.

Suggested change
public matchCount?: number;
public matchCount: number = 0;

@bmschmidt bmschmidt changed the base branch from sort-selections to graphite-base/143 September 20, 2024 03:12
@bmschmidt bmschmidt changed the base branch from graphite-base/143 to main September 20, 2024 03:14
@bmschmidt bmschmidt force-pushed the 08-07-make_the_tiles_on_a_selection_a_struct_of_info_not_twinned_lists branch from 3c779e2 to 927fc1c Compare September 20, 2024 03:17
let current_tile_ix = 0;
for (const match_length of this.match_count) {
const tile = this.tiles[current_tile_ix];
for (const { tile, matchCount } of this.tiles) {
Copy link

Choose a reason for hiding this comment

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

Ensure 'matchCount' is defined before using it in destructuring assignments. This applies to other instances like lines 728 and 642.

@bmschmidt bmschmidt merged commit d70b585 into main Sep 20, 2024
1 check passed
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.

3 participants