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

📂 Drop folders in pattern expansion #1399

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

agoose77
Copy link
Contributor

This PR addresses #1323 by:

  1. Simplifying patterns to only generate file entries
  2. Drop support for pattern entries with children -- in the schema, pattern entries can only be leaves.

@agoose77 agoose77 changed the title 📂 Drop folders in pattern expansin 📂 Drop folders in pattern expansino Jul 20, 2024
@agoose77 agoose77 changed the title 📂 Drop folders in pattern expansino 📂 Drop folders in pattern expansion Jul 20, 2024
@agoose77 agoose77 requested a review from fwkoch July 20, 2024 16:50
@agoose77 agoose77 force-pushed the agoose77/feat!-drop-pattern-folders branch from 0888049 to 4db5344 Compare July 22, 2024 18:28
Copy link
Collaborator

@fwkoch fwkoch left a comment

Choose a reason for hiding this comment

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

Only one critical change (warn about ignored children) - but I'd also love to see some of the tests persist, so the new behavior is clear.

packages/myst-cli/src/project/fromTOC.ts Show resolved Hide resolved
packages/myst-cli/src/project/fromTOC.ts Outdated Show resolved Hide resolved
@rowanc1 rowanc1 changed the title 📂 Drop folders in pattern expansion 📂 Drop folders in pattern expansion Jul 22, 2024
@agoose77 agoose77 force-pushed the agoose77/feat!-drop-pattern-folders branch from 555e8a4 to d00232c Compare July 23, 2024 12:51
obj[part] = objFromPathParts(remainingParts, obj[part]);
return obj;
}
export function comparePaths(a: string, b: string): number {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fwkoch this function now performs the directory-sensitive string comparison:

  1. Paths with non-matching directories are nat-sorted
  2. Paths with matching directories are nat-sorted if neither file is an index, or both files are index files
  3. Paths with non-matching directories are sorted in preference of the index file if one file is an index

Does this match your expectations?

@agoose77 agoose77 force-pushed the agoose77/feat!-drop-pattern-folders branch from d00232c to 2abda04 Compare July 23, 2024 12:56
Copy link
Collaborator

@fwkoch fwkoch left a comment

Choose a reason for hiding this comment

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

I like this much better. There's still a couple subtle differences between pattern expansion and implicit toc construction, but I don't think they matter? Just wanted to mention them so we are clear that the differences are intentional.

packages/myst-cli/src/project/fromTOC.ts Show resolved Hide resolved
packages/myst-cli/src/project/fromTOC.ts Show resolved Hide resolved
packages/myst-cli/src/project/fromTOC.ts Show resolved Hide resolved
packages/myst-cli/src/project/fromTOC.ts Show resolved Hide resolved
@agoose77 agoose77 merged commit ee0378e into main Jul 25, 2024
7 checks passed
@agoose77 agoose77 deleted the agoose77/feat!-drop-pattern-folders branch July 25, 2024 09:00
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