Ticket #268 (closed enhancement: fixed)

Opened 10 years ago

Last modified 9 years ago

[PATCH] Allow using SI-based size prefixes

Reported by: bilbo Owned by: angel_il
Priority: minor Milestone: 4.7.0-pre2
Component: mc-core Version: master
Keywords: SI Cc:
Blocked By: Blocking:
Branch state: Votes for changeset: committed-master

Description

I have created a patch that allows switching between old (default) 1024 based "binary" prefixes and SI (1000 based) size prefixes when displaying file sizes. I personally prefer 1000-based prefixes, so this patch gives user a choice to use them too if they want.

Attachments

kilobyte-git.patch (4.8 KB) - added by bilbo 10 years ago.
Patch to allow using SI units for byte size prefixes
kilobyte-git.2.patch (4.8 KB) - added by bilbo 10 years ago.
fixed patch, the condition is now correct
kilobyte-git.3.patch (4.8 KB) - added by bilbo 10 years ago.
Updated to current git master
kilobyte-git.4.patch (4.2 KB) - added by bilbo 9 years ago.
Update to current master
kilobyte-git.5.patch (5.9 KB) - added by bilbo 9 years ago.
Added configuration for this option into dialog

Change History

Changed 10 years ago by bilbo

Patch to allow using SI units for byte size prefixes

comment:1 Changed 10 years ago by bilbo

  • Keywords review added

comment:2 Changed 10 years ago by bilbo

  • Owner set to bilbo
  • Status changed from new to accepted

comment:3 Changed 10 years ago by slyfox

  • Keywords rework added; review removed

Look here:

279 /*
280 * If true, SI units (1000 based) will be used for
281 * larger units (kilobyte, megabyte, ...).
282 * If false binary units (1024 based) will be used.
283 */
284 bool kilobyte_si = 0;

and here:

393 if (kilobyte_si) {
394 size = (size + 512) >> 10;
395 } else {
396 size = (size + 500) / 1000;
397 }

Looks like condition shoud be inverted.

The rest looks fine
(except, maybe, already been there nongettextized "M" and "K" duplication).

Changed 10 years ago by bilbo

fixed patch, the condition is now correct

comment:4 Changed 10 years ago by bilbo

Oops, I overlooked this. I have attached a fixed patch.

What do you mean by 'nongettextized "M" and "K" duplication' ?

comment:5 Changed 10 years ago by slyfox

What do you mean by 'nongettextized "M" and "K" duplication' ?

Ideally,

{"", "k", "m", "g", "t", "p", "e", "z", "y", NULL};

could be internationalized via gettext (in far future :])

and this looks like identifier duplication:

xtra = kilobyte_si?"k":"K";

...

xtra = kilobyte_si?"m":"M";

can be changed to suffix[1,2]/suffix_ic[1,2] (with little code restructurements: move suffix* definitions upper). But will it be more readable?

Tried to apply patch and didn't find this config option in menus. Is it intended for use in '~/.mc/ini' file only?

comment:6 Changed 10 years ago by metux

Added your patch to the 268_si_units branch @ git://git.metux.de/free-mc.git/

Please check back and let me know if evrything's fine for going to master.

Changed 10 years ago by bilbo

Updated to current git master

comment:7 Changed 10 years ago by bilbo

Seems the repository at git://git.metux.de/free-mc.git/ differ a lot to git://midnight-commander.org/git/mc.git - I am not sure which one is outdated though ....

I was unable to compile from the branch from git://git.metux.de/free-mc.git/ due to missing libraries, though I have updated the patch for current head from git://midnight-commander.org/git/mc.git and it works fine.

comment:8 Changed 10 years ago by slavazanko

we don't support git://git.metux.de/free-mc.git/ repro, this repro was created by Enrico Weigelt (aka Metux)some time ago, where we drop from sourses tree support of mhl.

I don't know, is this repro actual for now.

Just use our repro, please :)

comment:9 Changed 10 years ago by slavazanko

attachment kilobyte-git.3.patch added

And big thanks for patch - we will review it in near time.

Changed 9 years ago by bilbo

Update to current master

comment:10 Changed 9 years ago by bilbo

  • severity set to no branch

Updated patch to current master - no actual changes, but removed parts of the patch that changed the changelog, which is no longer present in the source tree.

Changed 9 years ago by bilbo

Added configuration for this option into dialog

comment:11 Changed 9 years ago by bilbo

  • Keywords review added; rework removed
  • Version changed from 4.6.2 to master

I have reworked the patch:

The definition of kilobyte_si variable is moved to dir.c where it seems to be more logical along with variables like show_dot_files and show_backup_files

I've added possibility to configure this setting in dialog (Option -> Configuration -> Panel options -> SI size prefixes)

As a result of this change, the section in .mc/ini file where this setting is stored changed from [Misc] to [Midnight-Commander]

You can now toggle this settings in runtime, so you no longer need to manually alter the .mc/ini file.

Please review the patch with and consider it for inclusion in master.

comment:12 Changed 9 years ago by angel_il

  • Keywords SI added; review removed
  • Owner changed from bilbo to angel_il
  • Votes for changeset set to angel_il
  • severity changed from no branch to on review
  • Milestone changed from 4.7 to 4.7.0-pre3

look good, my vote here

comment:13 Changed 9 years ago by angel_il

branch: 268_si_based_size (parent: master)

comment:14 Changed 9 years ago by angel_il

comment:15 Changed 9 years ago by andrew_b

changeset:190c66e0e09364db3713c560eccfbf25fcf229b7

Changed option name in 'Configuration options' dialog.
Moved description to proper section in man page.
Added russian translation.

comment:16 Changed 9 years ago by slavazanko

  • Votes for changeset changed from angel_il to angel_il slavazanko
  • severity changed from on review to approved

comment:17 Changed 9 years ago by angel_il

  • Status changed from accepted to testing
  • Votes for changeset changed from angel_il slavazanko to commited-master
  • severity changed from approved to merged
  • Priority changed from major to minor
  • Milestone changed from 4.7.0-pre3 to 4.7.0-pre2
  • Resolution set to fixed

comment:18 Changed 9 years ago by angel_il

  • Status changed from testing to closed
Note: See TracTickets for help on using tickets.