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

Implement semver comparison for rustic-doc dependencies #48

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

Conversation

Velnbur
Copy link

@Velnbur Velnbur commented Sep 28, 2024

Implement full comaparison for semver-like versions returned by pandoc and fd-find.

Closes brotzeit/rustic#502

@Velnbur Velnbur changed the title Implement semver comparison for deps Implement semver comparison for rustic-doc dependencies Sep 28, 2024
@psibi
Copy link
Member

psibi commented Sep 28, 2024

Thanks! Can you see why the tests are failing ?

@therealpxc
Copy link

Hey, thanks for moving this PR here! :)

Copy link

@therealpxc therealpxc left a comment

Choose a reason for hiding this comment

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

the changes described in my comments fix evaluation for me so that rustic-mode works.

(Unfortunately it turns out I actually can't test rustic-doc on my machine because I'm not using rustup to provide Rust. After this goes in, I'll have to open my own PR for that. :)

rustic-doc.el Outdated Show resolved Hide resolved
rustic-doc.el Outdated Show resolved Hide resolved
rustic-doc.el Outdated
(str-length (length splitted-str)))
(mapcar #'string-to-number
;; take only first 3 elements or less
(subseq splitted-str 0 (min 3 str-length)))))

Choose a reason for hiding this comment

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

subseq

This isn't in scope on my system and makes defun evaluation fail. This should be seq-subseq, right?

Copy link
Author

Choose a reason for hiding this comment

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

Interesting, by the time I made this PR, I had been using Doom Emacs, but my current configuration, that is made from scratch, doesn't have this function too. Maybe some of Doom Emacs dependencies added it to my environment. Added seq-subseq instead, tests seems working

Choose a reason for hiding this comment

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

(as it happens, I'm using Doom currently, and this change was still required for it to work on my system)

Implement full comaparison for semver-like versions returned by pandoc
and fd-find.
@Velnbur Velnbur force-pushed the fix/rustic-doc-versions-check-fork branch from 3401daf to 59353d1 Compare September 28, 2024 16:33
@Velnbur
Copy link
Author

Velnbur commented Sep 28, 2024

Thank you for your comments! I'm beginer at writing Emacs packages code, so really appreciate your comments!

Also, pandoc has this really strange non-compatible with semver version which consists of four parts instead of 3 (you can see example in tests), as most of users are using the third version (3...*) this should be okay, but in future may cause issues

@Velnbur
Copy link
Author

Velnbur commented Sep 28, 2024

(Unfortunately it turns out I actually can't test rustic-doc on my machine because I'm not using rustup to provide Rust. After this goes in, I'll have to open my own PR for that. :)

As a NixOS user, I have this kind of issues too. I planned modifing current behavior by setting path to rust-doc by myself, but I currently don't have time for that 🥲

@therealpxc
Copy link

As a NixOS user, I have this kind of issues too. I planned modifing current behavior by setting path to rust-doc by myself, but I currently don't have time for that 🥲

I started a discussion about related issues here. It'll tag you as a reviewer when I start that work! That way you can at least follow along and we can learn together, even if you don't have time to work on it yourself.

@psibi
Copy link
Member

psibi commented Sep 29, 2024

@therealpxc Since this PR is assigned to you, let me know when it's ready to merge. I don't use rustic-doc and I think someone who uses it can review it better.

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.

version check issues?
3 participants