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

FileIdCache: Allow flexible handle instead of direct borrow for file ids #664

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

florian-g2
Copy link

Hi,
I suggest changing the return type of the cached_file_id method in the FileIdCache from Option<&FileId> to Option<impl Deref<Target=FileId>>; (breaking change)

This allows trait consumers to choose more freely how to return the FileId.
In a FileIdCache implementation I had to do I was required to return a owned copy of the file id.
The changed return type allows eg. a Cow::Owned or any other new-type implementation that dereferences to a FileId.

This is now doable with the recent MSRV raise to 1.77.

@dfaust
Copy link
Member

dfaust commented Jan 8, 2025

I'm split on this. FileId is just 256 bytes and implements Copy. So maybe returning Option<FileId> would be easiest. But I'm willing to merge it, if you think it's an improvement.
Please update the change-log as well.

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