From b8487a7c1cd77e4707d9954708e5fb23067509c2 Mon Sep 17 00:00:00 2001 From: Fienny Angelina Date: Tue, 16 Oct 2018 00:20:05 +0800 Subject: [PATCH] Refactor : - s/getGitlastupdated/getGitLastUpdatedTime - refactor part in getGitLastUpdated[Time|By] that overlaps - remove getAuthorInformation --- v1/lib/core/DocsLayout.js | 4 +- v1/lib/core/__tests__/utils.test.js | 16 ++-- v1/lib/core/utils.js | 128 ++++++++++------------------ 3 files changed, 57 insertions(+), 91 deletions(-) diff --git a/v1/lib/core/DocsLayout.js b/v1/lib/core/DocsLayout.js index e095e4b8e86a5..a67ee7878d46c 100644 --- a/v1/lib/core/DocsLayout.js +++ b/v1/lib/core/DocsLayout.js @@ -17,7 +17,7 @@ const OnPageNav = require('./nav/OnPageNav.js'); const Site = require('./Site.js'); const translation = require('../server/translation.js'); const docs = require('../server/docs.js'); -const {idx, getGitLastUpdated, getGitLastUpdatedBy} = require('./utils.js'); +const {idx, getGitLastUpdatedTime, getGitLastUpdatedBy} = require('./utils.js'); // component used to generate whole webpage for docs, including sidebar/header/footer class DocsLayout extends React.Component { @@ -48,7 +48,7 @@ class DocsLayout extends React.Component { const filepath = docs.getFilePath(metadata); const updateTime = this.props.config.enableUpdateTime - ? getGitLastUpdated(filepath) + ? getGitLastUpdatedTime(filepath) : null; const updateAuthor = this.props.config.enableUpdateBy ? getGitLastUpdatedBy(filepath) diff --git a/v1/lib/core/__tests__/utils.test.js b/v1/lib/core/__tests__/utils.test.js index 79f0128efb0b7..9e71e7d0ad46c 100644 --- a/v1/lib/core/__tests__/utils.test.js +++ b/v1/lib/core/__tests__/utils.test.js @@ -74,10 +74,10 @@ describe('utils', () => { expect(utils.removeExtension('pages.js')).toBe('pages'); }); - test('getGitLastUpdated', () => { + test('getGitLastUpdatedTime', () => { // existing test file in repository with git timestamp const existingFilePath = path.join(__dirname, '__fixtures__', 'test.md'); - const gitLastUpdated = utils.getGitLastUpdated(existingFilePath); + const gitLastUpdated = utils.getGitLastUpdatedTime(existingFilePath); expect(typeof gitLastUpdated).toBe('string'); expect(Date.parse(gitLastUpdated)).not.toBeNaN(); expect(gitLastUpdated).not.toBeNull(); @@ -88,14 +88,14 @@ describe('utils', () => { '__fixtures__', '.nonExisting', ); - expect(utils.getGitLastUpdated(null)).toBeNull(); - expect(utils.getGitLastUpdated(undefined)).toBeNull(); - expect(utils.getGitLastUpdated(nonExistingFilePath)).toBeNull(); + expect(utils.getGitLastUpdatedTime(null)).toBeNull(); + expect(utils.getGitLastUpdatedTime(undefined)).toBeNull(); + expect(utils.getGitLastUpdatedTime(nonExistingFilePath)).toBeNull(); // temporary created file that has no git timestamp const tempFilePath = path.join(__dirname, '__fixtures__', '.temp'); fs.writeFileSync(tempFilePath, 'Lorem ipsum :)'); - expect(utils.getGitLastUpdated(tempFilePath)).toBeNull(); + expect(utils.getGitLastUpdatedTime(tempFilePath)).toBeNull(); fs.unlinkSync(tempFilePath); // test renaming and moving file @@ -115,7 +115,7 @@ describe('utils', () => { '\n' + ' create mode 100644 v1/lib/core/__tests__/__fixtures__/.temp2\n', })); - const createTime = utils.getGitLastUpdated(tempFilePath2); + const createTime = utils.getGitLastUpdatedTime(tempFilePath2); expect(typeof createTime).toBe('string'); // rename / move the file @@ -128,7 +128,7 @@ describe('utils', () => { '\n' + ' create mode 100644 v1/lib/core/__tests__/__fixtures__/.temp2\n', })); - const lastUpdateTime = utils.getGitLastUpdated(tempFilePath3); + const lastUpdateTime = utils.getGitLastUpdatedTime(tempFilePath3); // should only consider file content change expect(lastUpdateTime).toEqual(createTime); }); diff --git a/v1/lib/core/utils.js b/v1/lib/core/utils.js index 94ee075460dcc..7475c9a338541 100644 --- a/v1/lib/core/utils.js +++ b/v1/lib/core/utils.js @@ -6,8 +6,6 @@ */ const shell = require('shelljs'); -const execSync = require('child_process').execSync; - const TRUNCATE_MARKER = //; function blogPostHasTruncateMarker(content) { @@ -40,121 +38,89 @@ function idx(target, keyPaths) { ); } -function getGitLastUpdated(filepath) { - function isTimestamp(str) { - return /^\d+$/.test(str); +const GIT_LAST_UPDATED_TYPE = { + TIME: 'time', + AUTHOR: 'author', +}; + +function getGitLastUpdated(filepath, type) { + if (Object.values(GIT_LAST_UPDATED_TYPE).indexOf(type) === -1) { + return null; } // Wrap in try/catch in case the shell commands fail (e.g. project doesn't use Git, etc). try { + const format = GIT_LAST_UPDATED_TYPE.TIME === type ? '%ct' : 'author=%an'; // To differentiate between content change and file renaming / moving, use --summary // To follow the file history until before it is moved (when we create new version), use // --follow. const silentState = shell.config.silent; // Save old silent state. shell.config.silent = true; const result = shell - .exec(`git log --follow --summary --format=%ct ${filepath}`) + .exec(`git log --follow --summary --format=${format} ${filepath}`) .stdout.trim(); shell.config.silent = silentState; - // Format the log results to be + // Format the log results to be either // ['1234567', 'rename ...', '1234566', 'move ...', '1234565', '1234564'] + // OR + // ['Yangshun Tay', 'rename ...', 'Endiliey', 'move ...', 'Joel', 'Fienny'] const records = result .toString('utf-8') .replace(/\n\s*\n/g, '\n') .split('\n') .filter(String); - - const timeSpan = records.find((item, index, arr) => { - const currentItemIsTimestamp = isTimestamp(item); - const isLastTwoItem = index + 2 >= arr.length; - const nextItemIsTimestamp = isTimestamp(arr[index + 1]); - return currentItemIsTimestamp && (isLastTwoItem || nextItemIsTimestamp); - }); - - if (timeSpan) { - const date = new Date(parseInt(timeSpan, 10) * 1000); - return date.toLocaleString(); - } - } catch (error) { + return records; + } catch(error) { console.error(error); } + return null; +} +function getGitLastUpdatedTime(filepath) { + function isTimestamp(str) { + return /^\d+$/.test(str); + } + + const records = getGitLastUpdated(filepath, GIT_LAST_UPDATED_TYPE.TIME); + const timeSpan = records.find((item, index, arr) => { + const currentItemIsTimestamp = isTimestamp(item); + const isLastTwoItem = index + 2 >= arr.length; + const nextItemIsTimestamp = isTimestamp(arr[index + 1]); + return currentItemIsTimestamp && (isLastTwoItem || nextItemIsTimestamp); + }); + + if (timeSpan) { + const date = new Date(parseInt(timeSpan, 10) * 1000); + return date.toLocaleString(); + } return null; } function getGitLastUpdatedBy(filepath) { function isAuthor(str) { - return str.startsWith('author='); - } - // Wrap in try/catch in case the shell commands fail (e.g. project doesn't use Git, etc). - try { - const silentState = shell.config.silent; // save old silent state - shell.config.silent = true; - const result = shell - .exec(`git log --follow --summary --format=author=%an ${filepath}`) - .stdout.trim(); - shell.config.silent = silentState; - // Format the log results to be ['1234567', 'rename ...', '1234566', 'move ...', '1234565', '1234564'] - const records = result - .toString('utf-8') - .replace(/\n\s*\n/g, '\n') - .split('\n') - .filter(String); - const lastContentModifierAuthor = records.find((item, index, arr) => { - const currentItemIsAuthor = isAuthor(item); - const isLastTwoItem = index + 2 >= arr.length; - const nextItemIsAuthor = isAuthor(arr[index + 1]); - return currentItemIsAuthor && (isLastTwoItem || nextItemIsAuthor); - }); - if (lastContentModifierAuthor) { - return lastContentModifierAuthor.slice(7); - } - } catch (error) { - console.error(error); + return str && str.startsWith('author='); } - return null; -} + const records = getGitLastUpdated(filepath, GIT_LAST_UPDATED_TYPE.AUTHOR); -function getAuthorInformation(filepath) { - /* set metadata author */ - const authorRegex = /(\d+) author (.+)$/g; - const results = execSync( - `git blame --line-porcelain ${filepath} \ - | grep -I "^author " | sort | uniq -c | sort -nr; \ - `, - ) - .toString() - .split('\n'); - - const authors = []; - let totalLineCount = 0; - - /* handle case where it's not github repo */ - if (results.length && results[0].length) { - let authorData; - results.forEach(result => { - if ((authorData = authorRegex.exec(result)) !== null) { - const lineCount = parseInt(authorData[1], 10); - const name = authorData[2]; - authors.push({ - lineCount, - name, - }); - totalLineCount += lineCount; - } - authorRegex.lastIndex = 0; - }); + const lastContentModifierAuthor = records.find((item, index, arr) => { + const currentItemIsAuthor = isAuthor(item); + const isLastTwoItem = index + 2 >= arr.length; + const nextItemIsAuthor = isAuthor(arr[index + 1]); + return currentItemIsAuthor && (isLastTwoItem || nextItemIsAuthor); + }); + if (lastContentModifierAuthor) { + return lastContentModifierAuthor.slice(7); } - return {authors, totalLineCount}; + + return null; } module.exports = { blogPostHasTruncateMarker, extractBlogPostBeforeTruncate, - getGitLastUpdated, - getAuthorInformation, + getGitLastUpdatedTime, getGitLastUpdatedBy, getPath, removeExtension,