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

Allow changing the Display of Range #221

Closed
konstin opened this issue May 10, 2024 · 4 comments
Closed

Allow changing the Display of Range #221

konstin opened this issue May 10, 2024 · 4 comments

Comments

@konstin
Copy link
Member

konstin commented May 10, 2024

In python, versions and version specifiers are formatted in particular way (PEP 440) that's different from pubgrub's default. We want to use the pubgrub::range::Range type for correctness and performance, but we'd like to override its Display impl. This is currently not possible with the newtype pattern due to segments of Range being private.

For reference, this is Display impl we want:

impl<V: Display + Eq> Display for Range<V> {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        if self.segments.is_empty() {
            write!(f, "∅")?;
        } else {
            for (idx, segment) in self.segments.iter().enumerate() {
                if idx > 0 {
                    write!(f, " | ")?;
                }
                match segment {
                    (Unbounded, Unbounded) => write!(f, "*")?,
                    (Unbounded, Included(v)) => write!(f, "<={v}")?,
                    (Unbounded, Excluded(v)) => write!(f, "<{v}")?,
                    (Included(v), Unbounded) => write!(f, ">={v}")?,
                    (Included(v), Included(b)) => {
                        if v == b {
                            write!(f, "=={v}")?
                        } else {
                            write!(f, ">={v}, <={b}")?
                        }
                    }
                    (Included(v), Excluded(b)) => write!(f, ">={v}, <{b}")?,
                    (Excluded(v), Unbounded) => write!(f, ">{v}")?,
                    (Excluded(v), Included(b)) => write!(f, ">{v}, <={b}")?,
                    (Excluded(v), Excluded(b)) => write!(f, ">{v}, <{b}")?,
                };
            }
        }
        Ok(())
    }
}
@zanieb
Copy link
Member

zanieb commented May 10, 2024

Can we just add it to the report formatter?

@mpizenberg
Copy link
Member

We didn’t make the internal representation of range public because it is easy to break guaranties if you have access to it. If I’m not mistaken though there is an iterator that gives lets you iterate over segments bounds. Could you use that to make a custom function to convert to a string?

@Eh2406
Copy link
Member

Eh2406 commented May 10, 2024

Isn't this exactly what #187 was intended to be used for? In your sample code self.segments.iter() can be replaced with self.iter() and it should all work.

@konstin
Copy link
Member Author

konstin commented May 13, 2024

Sorry, i missed the iter() for not returning Interval bit (Bound, Bound) 🤦. I've put up a half-baked branch at astral-sh/uv@main...konsti/pubgrub-range, the main task and verbosity is to forward all the utility methods of Range (Maybe something for a trait? Feels a bit specific though).

@konstin konstin closed this as completed May 13, 2024
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

No branches or pull requests

4 participants