Automatically generate rustdocs links to references in markdown#74
Automatically generate rustdocs links to references in markdown#74m4tx merged 16 commits intocot-rs:masterfrom
Conversation
| match parts.len() { | ||
| 1 => format!("{}.0.0", parts[0]), | ||
| 2 => format!("{}.{}.0", parts[0], parts[1]), | ||
| _ => s.to_string(), |
There was a problem hiding this comment.
Won't SemVer err if there are more than 4 parts? If so, I'd throw an error in that case and add tests coverage
There was a problem hiding this comment.
The minimum number of parts that Semver crate supports is 3. This is only a best effort to canonicalize any format less than 3 to have the standard major, minor, and patch. For cases > than 3, the idea is to route them to the SemVer::Version::parse constructor, which should validate them and throw an error when necessary
| } | ||
| } | ||
|
|
||
| fn canonicalize_version_string(s: &str) -> String { |
There was a problem hiding this comment.
I'm not exactly sure if this is the way we want to handle this. Suppose a user sees 0.5 version of the framework, but the latest version is 0.5.31. The URL generated will point specifically to 0.5.0, possibly omitting the changes made in the patch versions.
docs.rs is capable of redirecting to the latest patch version by itself, maybe we should use that instead? For instance, see that https://docs.rs/cot/0.3/cot/ redirects to 0.3.1, not 0.3.0.
I'm not even sure we need to parse the version. The content changes rarely and it's easy to spot the version might be wrong. Therefore, I think keeping it as String (wrapped in the Version newtype) would be enough, but I'll leave it to you to decide.
There was a problem hiding this comment.
I probably must have missed something initially while playing around the rustdocs link structure using the semver versioning which didnt work initially and influenced this function. But, you're right. I tried this again and it works. Which means we dont need this at all
There was a problem hiding this comment.
Actually, I just remembered another reason for this function. The semver crate requires at least a three-component version (major, minor, patch) to construct a valid smever::Version type. However, throughout the codebase, versions are typically specified with only the major and minor components.
To accommodate this, we canonicalize to the “floor” (i.e., append a patch component, typically 0) as a best-effort strategy to produce a valid semver::Version when the patch component is not explicitly provided.
I think it makes sense to retain this behavior, and then when generating the Rustdoc link, we can use only the major and minor components. Rustdoc should then resolve the appropriate route to the latest compatible patch version, as you pointed out.
| /// Returns the major version number. | ||
| /// | ||
| /// # Example | ||
| /// ``` | ||
| /// let v = Version::new(0, 5, 0); | ||
| /// assert_eq!(v.major(), 0); | ||
| /// ``` | ||
| pub fn major(&self) -> u64 { | ||
| self.0.major | ||
| } | ||
|
|
||
| /// Returns the minor version number. | ||
| /// | ||
| /// # Example | ||
| /// ``` | ||
| /// let v = Version::new(0, 5, 0); | ||
| /// assert_eq!(v.minor(), 5); | ||
| /// ``` | ||
| pub fn minor(&self) -> u64 { | ||
| self.0.minor | ||
| } | ||
|
|
||
| /// Returns the patch version number. | ||
| /// | ||
| /// # Example | ||
| /// ``` | ||
| /// let v = Version::new(0, 5, 0); | ||
| /// assert_eq!(v.patch(), 0); | ||
| /// ``` | ||
| pub fn patch(&self) -> u64 { | ||
| self.0.patch | ||
| } |
There was a problem hiding this comment.
Do we use these methods anywhere? If not, there's no point in keeping dead code on the repo.
|
A long-awaited and very useful change, thanks! I think it's a little bit overengineered, though - please have a look at my comments. |
|
oops, didn't mean to close, sorry. |
…k-gen # Conflicts: # cot-site-macros/src/md_pages/rendering.rs
| segs.iter() | ||
| .skip(1) | ||
| .take(segs.len() - 2) | ||
| .map(|s| s.to_string()), |
There was a problem hiding this comment.
Please add a comment explaining why are we taking 1..n-2 segments, as it's not immediately obvious when looking at the code.
m4tx
left a comment
There was a problem hiding this comment.
LGTM, just address the last final comment!
Add logic to resolve references into valid rustdoc links