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

[feature] include files linked in the README #5

Open
PicoJr opened this issue May 31, 2020 · 5 comments
Open

[feature] include files linked in the README #5

PicoJr opened this issue May 31, 2020 · 5 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@PicoJr
Copy link

PicoJr commented May 31, 2020

Hello,

given the following dummy cargo layout:

.
├── Cargo.lock
├── Cargo.toml
├── dummy.txt
├── img
│  └── dummy.png
├── README.md
├── src
│  └── main.rs
└── target

and the following README.md:

───────┬──────────────────────────────────────────────
       │ File: README.md
───────┼──────────────────────────────────────────────
   1   │ # Dummy
   2   │ 
   3   │ ![dummy.png](img/dumy.png)
───────┴──────────────────────────────────────────────

Cargo diet does not keep the img/dummy.png.

❯ cargo diet -n
┌───────────────┬─────────────┐
│ File          │ Size (Byte) │
├───────────────┼─────────────┤
│ img/dummy.png │           0 │
│ .gitignore    │           8 │
│ dummy.txt     │        1998 │
└───────────────┴─────────────┘
Saved 87% or 2.0 KB in 3 files (of 2.3 KB and 6 files in entire crate)

The following change WOULD be made to Cargo.toml:
Diff - removed / added + :
[…skipped 3 lines…]

edition = "2018"
+include = ["src/main.rs", "README.md"]

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[…skipped 3 lines…]

Assuming the readme field is set to README.md it means the crate's README on crates.io will have a broken link.

Would it be possible to also include local files linked in the README?

It would require tokenizing the README, extract urls, filter out non local urls, add those urls to "include".

What do you think?

@Byron Byron added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Jun 1, 2020
@Byron
Copy link
Collaborator

Byron commented Jun 1, 2020

Thanks for this wonderfully written issue!

I agree, this is definitely something it should be able to do, especially knowing that it already tries very hard to pickup files mentioned in build.rs, lib.rs and all binaries mentioned in the Cargo.toml.

Skipping configured README files is an oversight.

As cargo-diet is just a frontend to criner-waste-report, the actual feature would have to implemented there.

The capability would have to be implemented in this function, probably somewhat similar to what it does to handle build scripts. (In the latter, paths are extracted using a regex, ignoring false positives).

In order to get started, it would be beneficial to have another unit test for this kind of package, which makes iterating on the code very fast and convenient. Currently there is no nice way of getting these binary packages though, but I will add one using cargo diet in a bit.

The implementation can probably use leverage a 'stupid' regex and ignore false positives similarly to how it's done in case of build scripts or lib.rs and binaries code files.

@Byron
Copy link
Collaborator

Byron commented Jun 1, 2020

Alright, the respective feature was added, and I wrote down everything I could imagine useful if somebody would want contribute in the corresponding guide.

Please let me know if you feel like giving it a shot, and I am happy to provide further assistance.

@PicoJr
Copy link
Author

PicoJr commented Jun 1, 2020

It sounds interesting, I might well giving it a shot but not in the near future: I got injured yesterday, I won't be coding stuff for a little while until it's healed.

@PicoJr PicoJr closed this as completed Jun 1, 2020
@PicoJr PicoJr reopened this Jun 1, 2020
@PicoJr
Copy link
Author

PicoJr commented Jun 1, 2020

clicked "close issue" instead of "comment"

@Byron
Copy link
Collaborator

Byron commented Jun 2, 2020

Thanks for letting me know, and all the best for the healing process!

As I find making cargo-diet better so very rewarding, I will probably end up implementing it myself 😅, will keep you posted here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants