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

feat: add last contributor to each document #980

Merged
merged 17 commits into from
Oct 18, 2018
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/api-site-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ Set this to `true` if you want to enable Facebook comments at the bottom of your
#### `fonts` [object]
Font-family CSS configuration for the site. If a font family is specified in `siteConfig.js` as `$myFont`, then adding a `myFont` key to an array in `fonts` will allow you to configure the font. Items appearing earlier in the array will take priority of later elements, so ordering of the fonts matter.

#### `enableUpdateBy` [boolean]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a wrong place to put this. Now it's in between the fonts config.

An option to enable the docs showing last update time. Set to `true` to show a line at the bottom right corner of each doc page as `Last Updated By: <Author Name>`.

In the below example, we have two sets of font configurations, `myFont` and `myOtherFont`. `Times New Roman` is the preferred font in `myFont`. `-apple-system` is the preferred in `myOtherFont`.

```js
Expand Down
3 changes: 3 additions & 0 deletions v1/examples/basics/siteConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ const siteConfig = {
ogImage: 'img/docusaurus.png',
twitterImage: 'img/docusaurus.png',

// Show documentation's contributors name.
// enableUpdateBy: true,

// You may provide arbitrary config keys to be used as needed by your
// template. For example, if you need your repo's URL...
// repoUrl: 'https://github.com/facebook/test-site',
Expand Down
34 changes: 23 additions & 11 deletions v1/lib/core/DocsLayout.js
Original file line number Diff line number Diff line change
Expand Up @@ -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} = 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 {
Expand Down Expand Up @@ -45,16 +45,19 @@ class DocsLayout extends React.Component {
if (this.props.Doc) {
DocComponent = this.props.Doc;
}
const filepath = docs.getFilePath(metadata);

let updateTime;
if (this.props.config.enableUpdateTime) {
const filepath = docs.getFilePath(metadata);
updateTime = getGitLastUpdated(filepath);
}
const updateTime = this.props.config.enableUpdateTime
? getGitLastUpdatedTime(filepath)
: null;
const updateAuthor = this.props.config.enableUpdateBy
? getGitLastUpdatedBy(filepath)
: null;

const title =
idx(i18n, ['localized-strings', 'docs', id, 'title']) || defaultTitle;
const hasOnPageNav = this.props.config.onPageNav === 'separate';

const previousTitle =
idx(i18n, ['localized-strings', metadata.previous_id]) ||
idx(i18n, ['localized-strings', 'previous']) ||
Expand Down Expand Up @@ -90,15 +93,24 @@ class DocsLayout extends React.Component {
version={metadata.version}
language={metadata.language}
/>
{this.props.config.enableUpdateTime &&
updateTime && (
<div className="docLastUpdateTimestamp">
{(updateTime || updateAuthor) && (
<div className="docLastUpdate">
{updateTime && (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use a combined format:

  • Only enableUpdateTime: "Last updated on 2018-10-15 01:23"
  • Only enableUpdateBy: "Last updated by @fiennyangeln"
  • Both enableUpdateTime and enableUpdateBy: "Last updated on 2018-10-15 01:23 by @fiennyangeln"

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's nice 😍 ! I will implement this.

<em>
<strong>Last updated: </strong>
{updateTime}
</em>
</div>
)}
)}
<br />
{updateAuthor && (
<em>
<strong>Last updated by: </strong>
{updateAuthor}
</em>
)}
</div>
)}

<div className="docs-prevnext">
{metadata.previous_id && (
<a
Expand Down
16 changes: 8 additions & 8 deletions v1/lib/core/__tests__/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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);
});
Expand Down
73 changes: 55 additions & 18 deletions v1/lib/core/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,53 +38,90 @@ 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';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the author here?

Let's do away with this format variable. Instead, we could do

git log --follow --summary --format="%ct, %an" <file>

which would generate something like:

1539553317, Yangshun Tay

1539457591, Yangshun Tay

1539185600, Yangshun Tay

1538339532, Fienny Angelina

1537426859, Marvin Heilemann

1537330146, Yangshun Tay

1537203487, Yangshun Tay

 copy package.json => v1/package.json (81%)
1537169695, Yangshun Tay

1537157761, Yangshun Tay

1537157671, Endilie Y

1537157183, endiliey

and then parse each line for the timestamp and author using a regex that extracts the timestamp and the author. Exampe regex here - https://regex101.com/r/O0HQrc/1

This method will just return both the relevant timestamp and author as an object { timestamp: 12345, author: 'Fienny' }, and it's up to the callsite to use whichever field they need.

Copy link
Contributor Author

@fiennyangeln fiennyangeln Oct 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially put the author to differentiate author vs commit summary since both are just plain string 😁 Wdyt about making it stronger like ^(\d{10}), (.+)$ ? It can last until 2038 based on Google

// 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();
}
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) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This finding logic can be shifted into the getGitLastUpdated method.

And getGitLastUpdatedTime will in the end just become:

function getGitLastUpdatedTime(filepath) {
  const commit = getGitLastUpdated(filepath);
  if (!commit) {
    return null;
  }

  const date = new Date(parseInt(commit.timestamp, 10) * 1000);
  return date.toLocaleString();
}

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 && str.startsWith('author=');
}

const records = getGitLastUpdated(filepath, GIT_LAST_UPDATED_TYPE.AUTHOR);

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 null;
}

module.exports = {
blogPostHasTruncateMarker,
extractBlogPostBeforeTruncate,
getGitLastUpdated,
getGitLastUpdatedTime,
getGitLastUpdatedBy,
getPath,
removeExtension,
idx,
Expand Down
2 changes: 1 addition & 1 deletion v1/lib/static/css/main.css
Original file line number Diff line number Diff line change
Expand Up @@ -1536,7 +1536,7 @@ input::placeholder {
}
}

.docLastUpdateTimestamp {
.docLastUpdate {
font-size: 13px;
font-style: italic;
margin: 20px 0;
Expand Down
1 change: 1 addition & 0 deletions v1/website/siteConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const siteConfig = {
zIndex: 100,
},
enableUpdateTime: true,
enableUpdateBy: true,
customDocsPath: '../docs',
};

Expand Down