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

dl: show the download progress in megabytes (mebibytes) instead of bytes #45556

Open
perillo opened this issue Apr 14, 2021 · 11 comments
Open

dl: show the download progress in megabytes (mebibytes) instead of bytes #45556

perillo opened this issue Apr 14, 2021 · 11 comments
Labels
Milestone

Comments

@perillo
Copy link
Contributor

@perillo perillo commented Apr 14, 2021

Showing the download progress in bytes is not very convenient. The current and total sizes should be in megabytes (mebibytes).

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Apr 14, 2021

Sorry, but I'm missing what exactly "dl" is. The only reference I can find is the subset of x/website that references the download page.

@perillo
Copy link
Contributor Author

@perillo perillo commented Apr 14, 2021

It is the golang.org/dl module: https://go.googlesource.com/dl

In some other issues is referenced as x/dl, but I'm not sure what is the correct reference.

@mknyszek mknyszek added this to the Unreleased milestone Apr 14, 2021
@mknyszek mknyszek changed the title dl: show the download progress in megabytes (mebibytes) instead of bytes x/dl: show the download progress in megabytes (mebibytes) instead of bytes Apr 14, 2021
@Hallicopter
Copy link

@Hallicopter Hallicopter commented Apr 14, 2021

This seems like a somewhat beginner-friendly issue to solve. (Have used Go a lot, but never contributed) I would like to help out in case nobody else is working on it.
Any pointers from where I can begin?
I will explore the package a bit myself in the meanwhile.

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 14, 2021

Change https://golang.org/cl/310016 mentions this issue: internal/version: show the download progress in mebibytes

@perillo
Copy link
Contributor Author

@perillo perillo commented Apr 14, 2021

@Hallicopter, I was sure to have already submitted the cl; sorry if you have already started working on it.

@Hallicopter
Copy link

@Hallicopter Hallicopter commented Apr 14, 2021

That's not a problem at all! Just wanted to contribute back to Go in some way since it has helped me so much. I can always find something else.

@slrz
Copy link

@slrz slrz commented Apr 14, 2021

Bytes is much nicer to re-use though. It's more precise and there is no possible confusion with regard to M vs. Mi unit prefixes. You can always convert to displaying MB using a trivial filter while the opposite direction is not possible (the proposed patch throws away information).

I don't think this change is a net improvement.

@perillo
Copy link
Contributor Author

@perillo perillo commented Apr 14, 2021

If it needs to be precise I can replace MB with MiB (as done by pacman, as an example).
About bytes being more precise, I'm not sure. But I'm sure that with the size in byte, it is harder to follow how much data is being downloaded (at least for me).

To make the progress more precise we can choose the best unit, between KiB and MiB (again, as it is done by pacman).

Thanks.

@perillo
Copy link
Contributor Author

@perillo perillo commented Apr 15, 2021

sample.txt shows the output with the latest patchset in https://golang.org/cl/310016.

The size is formatted in KiB without decimal digits when less then 2 MiB, and in MiB otherwise with 2 decimal digits. This ensures that the max width in KiB (2048) and the min width in MiB (2.00) are the same (4).

The code changes are minimal and simple.

@perillo
Copy link
Contributor Author

@perillo perillo commented Apr 15, 2021

Here is an alternative implementation:

func fmtsize(size int64) string {
	switch {
	case size < 10000:
		return strconv.FormatInt(size, 10) + " B"
	case size < 1000000:
		return strconv.FormatFloat(float64(size)/float64(KiB), 'f', 1, 64) + " KiB"
	}

	return strconv.FormatFloat(float64(size)/float64(MiB), 'f', 1, 64) + " MiB"
}

This works correctly only if the download size is greater than 100 MB (decimal).

@perillo
Copy link
Contributor Author

@perillo perillo commented Apr 15, 2021

This is probably the best version. The formatting is similar to the one in curl.

func fmtsize(size int64) string {
	// When formatted in KiB there are no decimal digits and when formatted in
	// MiB there is 1 decimal digit.  This ensures that the max width in KiB
	// and the min width in MiB are the same.
	switch {
	case size < 10*KiB:
		return strconv.FormatInt(int64(size), 10) + " B"
	case size < 10*MiB:
		return strconv.FormatFloat(float64(size)/float64(KiB), 'f', 0, 64) + " KiB"
	}

	return strconv.FormatFloat(float64(size)/float64(MiB), 'f', 1, 64) + " MiB"
}
@dmitshur dmitshur changed the title x/dl: show the download progress in megabytes (mebibytes) instead of bytes dl: show the download progress in megabytes (mebibytes) instead of bytes May 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants