Closed starlocke closed 5 years ago
Thanks for taking the time to improve Gitlogg!
I ran a diff
between the original output and the proposed output (the one I get after your changes).
I did it by running diff --speed-large-files output1.json output2.json
.
I observed some inconsistencies, so I isolated a commit. This is the output of an actual commit, which I anonymized.
// Output - original:
{"repository":"anonymized.git","commit_nr":116,"commit_hash":"7c4184283a6dfe624aa12d0a711fe1ad6792f869","author_name":"Anonimized","author_email":"anonymized@anonymized.com","author_date":"Fri Sep 2 11:37:08 2016 +0200","author_date_relative":"3 years, 1 month ago","author_date_unix_timestamp":"1472809028","author_date_iso_8601":"2016-09-02 11:37:08 +0200","subject":"anonymized","subject_sanitized":"anonymized","stats":" 1 file changed, 1 insertion(+), 1 deletion(-)","time_hour":11,"time_minutes":37,"time_seconds":8,"time_gmt":"+0200","date_day_week":"Fri","date_month_day":2,"date_month_name":"Sep","date_month_number":9,"date_year":"2016","date_iso_8601":"2016-09-02","files_changed":1,"insertions":1,"deletions":1,"impact":0};
// Output - proposed:
{"repository":"anonymized.git","commit_nr":116,"commit_hash":"7c4184283a6dfe624aa12d0a711fe1ad6792f869","author_name":"Anonimized","author_email":"anonymized@anonymized.com","author_date":"Fri Sep 2 11:37:08 2016 +0200","author_date_relative":"3 years, 1 month ago","author_date_unix_timestamp":"1472809028","author_date_iso_8601":"2016-09-02 11:37:08 +0200","subject":"anonymized","subject_sanitized":"anonymized","stats":" 1 file changed, 1 insertion(+), 1 deletion(-)","time_hour":11,"time_minutes":37,"time_seconds":8,"time_gmt":"+0200","date_day_week":"Fri","date_month_day":2,"date_month_name":"Sep","date_month_number":9,"date_year":"2016","date_iso_8601":"2016-09-02","files_changed":0,"insertions":0,"deletions":0,"impact":0};
So I decided to dig deeper, by reproducing the relevant part within gitlog-parse-json.js
:
// slice the string as long as it's not empty
const sliceit = (str) => {
if (str === undefined) {
return '';
} else if (str !== '') {
return str.slice(1);
}
return str;
};
// massage the stats
const changesOriginal = (data, index) => {
const v = data.split(',')[index] || 0;
let w = 0;
if (v !== 0) {
w = v.split(' ')[1]; // the number of changes is second on the array
}
return parseInt(w, 10);
};
// massage the stats
const changesProposed = (data, index) => {
let v = 0;
let m = null;
if (index === 0) {
// "files"
m = data.match(/ [0-9]+ files/);
if (m != null) v = m[0];
}
if (index === 1) {
// "insertions"
m = data.match(/ [0-9]+ insertions/);
if (m != null) v = m[0];
}
if (index === 2) {
// "deletions"
m = data.match(/ [0-9]+ deletions/);
if (m != null) v = m[0];
}
let w = 0;
if (v !== 0) {
w = v.split(' ')[1]; // the number of changes is second on the array
}
return parseInt(w, 10);
};
const returnStatsOriginal = (inp) => {
const stats = sliceit(inp);
const filesChanged = changesOriginal(stats, 0);
const insertions = changesOriginal(stats, 1);
const deletions = changesOriginal(stats, 2);
return `{
filesChanged: ${filesChanged},
insertions: ${insertions},
deletions: ${deletions},
}`;
};
const returnStatsProposed = (inp) => {
const stats = sliceit(inp);
const filesChanged = changesProposed(stats, 0);
const insertions = changesProposed(stats, 1);
const deletions = changesProposed(stats, 2);
return `{
filesChanged: ${filesChanged},
insertions: ${insertions},
deletions: ${deletions},
}`;
};
// Input:
const input = ' 1 file changed, 1 insertion(+), 1 deletion(-)';
console.log(returnStatsOriginal(input));
// {
// filesChanged: 1,
// insertions: 1,
// deletions: 1,
// }
console.log(returnStatsProposed(input));
// {
// filesChanged: 0,
// insertions: 0,
// deletions: 0,
// }
This didn't output correct values, but you were definitely in the right track as I could see that the original function failed with:
const input = ' 2827 files changed, 664411 deletions(-)';
// {
// filesChanged: NaN,
// insertions: 664411,
// deletions: 0,
// }
Testing it locally with Quokka.js, I was able to do further changes and verify them. After a few changes the function now works as intended. Thanks a lot for your contribution!
@starlocke these are my changes, for reference: https://github.com/dreamyguy/gitlogg/commit/0bb7b25b3b47e073ebf998e830fe8b7956ed6556. Thanks again for the PR! 🙌
The original
changes()
method did not do a proper extraction when a commit contained ONLY deletions. This remedies that.