sindrets / diffview.nvim

Single tabpage interface for easily cycling through diffs for all modified files for any git rev.
Other
3.74k stars 105 forks source link

[bug,mercurial] File history shows wrong stats #366

Closed sindrets closed 1 year ago

sindrets commented 1 year ago

The diff stats displayed in the file history panel are wrong for mercurial repos. The file stats for hg history is calculated by counting the sigils in the diffstat summary:

https://github.com/sindrets/diffview.nvim/blob/bff58a6ea3e081ca50049700f9848b2f84ea57be/lua/diffview/vcs/adapters/hg/init.lua#L753-L760

This is wrong because the sigils represent a percentage of the total number of changes in the file. Counting the sigils will be highly inaccurate for any file that has even a low number of changes.

@zegervdv Thoughts? Is there a way to get the actual number of insertions / deletions? If not I think we should not show stats at all for mercurial repos, because the way it's currently displayed is only misleading.

zegervdv commented 1 year ago

It seems to be correct for small changes (order of tens of lines) but I haven't checked yet for larger ones. I am working on an expansion of mercurials' output to be able to extract the actual data, but that will eventually only be available on newer mercurial versions.

I'll run some test with larger changes and if needed will look for a way to disable them and only show them when that expanded data is available.

sindrets commented 1 year ago

It's very unreliable first of all because it's treating the percentage as an absolute count. But further it will depend on what hg internally uses as the default max width for rendering the stat summary, and also the length of the paths present in the summary. Finally, the stat balance bar is scaled in relation to the other changed files in the commit. These properties combined will determine the resolution of the percentage (and it's always going to be low).

Take this example from a changeset in Mozilla's comm-central repo:

changeset:   38877:108be40cba38
user:        John Bieling <john@thunderbird.net>
date:        Sun May 07 10:20:28 2023 +0000
summary:     Bug 1817881 - Try to fix browser_ext_bug1812530.js (and others) by removing race condition between load status and load event. r=mkmelin

 [...]
 mail/components/extensions/test/browser/head.js       |  62 +++++----
 mail/components/extensions/test/browser/head_menus.js |  26 +---
 15 files changed, 81 insertions(+), 172 deletions(-)

File history for head.js currently renders like this:

  M mail/components/extensions/test/browser/head.js 5, 57

The correct stats would be closer to:

$$ \begin{align} insertions = \lfloor \frac{5}{9} \times 62 \rceil = 34 \ deletions = \lfloor \frac{4}{9} \times 62 \rceil = 28 \end{align} $$

But that is also inaccurate. The resolution of the percentage we're working with here is simply too low, and the margin of error is only going to get worse the larger the change is.

zegervdv commented 1 year ago

@sindrets You're right. As soon as one of the files in the log has more than ~30 lines changed, the stats switch to some percentage. I had only tested in a dummy repo with some small commits :stuck_out_tongue:, and didn't think about it in an actual repo.

I'll set them to nil until I can find a better way to get that data out.