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

please drop dependency on common-path #3702

Open
WeepingClown13 opened this issue Oct 25, 2024 · 3 comments
Open

please drop dependency on common-path #3702

WeepingClown13 opened this issue Oct 25, 2024 · 3 comments

Comments

@WeepingClown13
Copy link

WeepingClown13 commented Oct 25, 2024

A bit of context first; I am packaging zellij for Debian, for a while now. There are tons of dependencies that had to be and has to be packaged first, as per Debian policy. Currently I have finished packaging all dependencies/dev-dependencies of zellij-utils, except common-path.

common-path is currently version 1.0.0, and the last update to it was 6 years ago. That much is quite common, but this one also points to an nonexistent repository as source. Examining the source code, there is only so much - non complex - logic existing there. with less than 50 lines of actual library code excluding tests. At that point, having it as a separate package becomes only a lot of maintenance burden. Examining the common-path usage in zellij, it only seems to be used at a single place in zellij-utils.

In conclusion, it'd make life a lot of easier for downstream packaging if this dependency can be avoided. And that leaves some questions.

  1. Can the usage of common-path itself easily be removed? Would that affect anything severely?
  2. Are there better or well maintained alternatives that could be used, if the utility it provides is a must have?
  3. Even better, is it possible to zellij itself to inline its own logic for this so that the utility is retrieved and it's one less dependency?

Please share your thoughts!

@imsnif
Copy link
Member

imsnif commented Oct 25, 2024

Hey, thank you for your efforts! I would be very happy to see Zellij packaged for Debian. common-path is only used in one place in the code base: https://github.com/zellij-org/zellij/blob/main/zellij-server/src/session_layout_metadata.rs#L252

We use it there to find the common path of terminal working directories when serializing a layout for the session resurrection. This code isn't overly complex but can potentially be error prone. I'm happy to accept a PR that does this manually instead using the STD lib's primitives. I guess it shouldn't be overly complex. Wdyt?

@WeepingClown13
Copy link
Author

WeepingClown13 commented Oct 25, 2024

For starters, it helps to know that there is support, thanks! I am no rust wizard and honestly all my rust writing is patching debian
packages. I wouldn't call myself the best person to do this, especially if you are worried about intricate matters xD That said, no
one doing it will eventually mean myself doing some patching downstream, so I'll try to whip up something later if no one more
qualified than me want to give it a try. On that note, why do you not want to use the stdlib primitives?

@imsnif
Copy link
Member

imsnif commented Oct 25, 2024

My apologies - I was a bit unclear. I would be happy to use the STD lib's primitives. :)

Right now I'm mostly concerned with getting the next release out the door, but once that's done - if you haven't gotten this done yet I'll try to whip something up. This honestly sounds like a 5-10 lines of code function to me, but I'm always open to having missed something.

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

2 participants