Skip to content
This repository was archived by the owner on Mar 11, 2025. It is now read-only.

Replace &PathBuf with &Path#602

Merged
djc merged 1 commit intoaskama-rs:mainfrom
Kijewski:pr-no-ref-pathbuf
Jan 24, 2022
Merged

Replace &PathBuf with &Path#602
djc merged 1 commit intoaskama-rs:mainfrom
Kijewski:pr-no-ref-pathbuf

Conversation

@Kijewski
Copy link
Copy Markdown
Member

PathBuf is to String like Path is to str, so &PathBuf is a pointer to a pointer. Clippy does not likes that.

In #600 clippy did notice the problem after some refactoring. This change is at least a little useful on its own.

vallentin
vallentin previously approved these changes Jan 12, 2022
Copy link
Copy Markdown
Collaborator

@vallentin vallentin left a comment

Choose a reason for hiding this comment

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

Looks good 👍

(But don't merge until @djc says it looks good too)

@djc
Copy link
Copy Markdown
Collaborator

djc commented Jan 12, 2022

The as_path() calls seem unidiomatic to me. In order of preference, which of &path, *path and path.as_ref() works?

@djc
Copy link
Copy Markdown
Collaborator

djc commented Jan 12, 2022

Ugh, that's still pretty ugly. I'll see if I can play with this tomorrow.

@Kijewski
Copy link
Copy Markdown
Member Author

I'd actually say I made it worse. :D

@djc
Copy link
Copy Markdown
Collaborator

djc commented Jan 23, 2022

Yeah, okay, would you mind going back to using as_ref() instead of all the double dereferencing?

PathBuf is to String like Path is to str, so `&PathBuf` is a pointer to
a pointer. Clippy does not likes that.
@djc djc merged commit bb7c60e into askama-rs:main Jan 24, 2022
@Kijewski Kijewski deleted the pr-no-ref-pathbuf branch January 24, 2022 12:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants