8000 GenericPath cleanup by s-ludwig · Pull Request #422 · vibe-d/vibe-core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

GenericPath cleanup #422

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jan 2, 2025
Merged

GenericPath cleanup #422

merged 4 commits into from
Jan 2, 2025

Conversation

s-ludwig
Copy link
Member
@s-ludwig s-ludwig commented Jan 2, 2025
  • Swaps the implementation and documentation of .normalize and .normalized
  • Avoids allocations when normalizing an already normalized path
  • Deprecates .Segment2 and associated methods

Geod24
Geod24 reviewed Jan 2, 2025
View reviewed changes
source/vibe/core/path.d

See `normalized` for a full description.
*/
void normalize()
Copy link
Contributor

Choose a reason for hiding this comment

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

No deprecation ?

Geod24
Geod24 approved these changes Jan 2, 2025
View reviewed changes
Copy link
Contributor
@Geod24 Geod24 left a comment

Choose a reason for hiding this comment

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

Neat

source/vibe/core/path.d Outdated

/** Replaces the path representation with its normalized form.

See `normalized` for a full description.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
See `normalized` for a full description.
This is a simple wrapper around `normalized` which should be preferred. See `normalized` for a full description.

If you don't want to deprecate it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've searched through the code base of a few projects and normalize() is being used in a surprising amount of cases, so I'd like to avoid breaking the API for this. I've adjusted the a bit wording here, but went with this solution.

@s-ludwig s-ludwig merged commit eb70e07 into master Jan 2, 2025
13 checks passed
@s-ludwig s-ludwig deleted the swap_path_normalzed branch January 2, 2025 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Geod24 Geod24 Geod24 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@s-ludwig @Geod24
0