generated from MetaMask/metamask-module-template
-
-
Notifications
You must be signed in to change notification settings - Fork 10
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
* Add `validate` command The validate command will validate the formatting of the changelog. If the `--rc` flag is used, it will also ensure that the current version is included as a release header, and that there are no unreleased changes. The CLI command displays a rudimentary diff if formatting problems are detected. This can be improved later with colours, and with highlighting within each line to show what has changed. It also doesn't yet highlight whitespace changes. The validation logic is also exposed via the API as a function called `validateChangelog`. It doesn't include the fancy diff output, but it does throw an error with all of the required information for someone to construct the same output. Tests have been added to comprehensively test the changelog validation. Closes #11 * Add additional validation error types All invalid changelog cases are now described by the `InvalidChangelogError` error type, which has sub-classes to describe each possible scenario. This allows distinguishing invalid changelog errors from all other unexpected errors. The CLI has been updated to take advantage of this; we now print a simpler error message if the changelog is invalid, rather than a stack trace. * Improve `generateDiff` The `generateDiff` function now has a comprehensive test suite. It has been updated to ensure that the resulting diff displays a notice about whether or not the file has a trailing newline. I started this journey by trying to ensure we were trimming each line properly. I found that `diff.diffLines` always returned change sets that ended in a newline, except when returning the very last change to a file that didn't terminate in a newline. So in order to ensure we were correctly showing all changes, I needed to add special handling for end-of-file changes. We could display this differently than I've chosen to here, but I figured showing what `git` shows was a good start. The `generateDiff` function now also ensures the correct context is shown in between each change. The dependency `outdent` was added to help with writing inline test fixtures. This saves us from having to come up with a name for each fixture and cross-reference them in each test. When adding that dependency, an ESLint error brought to my attention that we were accidentally including test files in our `files` property in `package.json`. It was updated to exclude test files, which resolved the ESLint error.
- Loading branch information
Showing
9 changed files
with
1,146 additions
and
32 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| const diff = require('diff'); | ||
|
|
||
| /** | ||
| * Splits string into lines, excluding the newline at the end of each | ||
| * line. The trailing newline is optional. | ||
| * @param {string} value - The string value to split into lines | ||
| * @returns {Array<string>} The lines, without trailing newlines | ||
| */ | ||
| function getTrimmedLines(value) { | ||
| const trimmedValue = value.endsWith('\n') | ||
| ? value.substring(0, value.length - 1) | ||
| : value; | ||
| return trimmedValue.split('\n'); | ||
| } | ||
|
|
||
| /** | ||
| * Generates a diff between two multi-line string files. The resulting diff | ||
| * shows any changes using '-' and '+' to indicate the "old" and "new" version | ||
| * respectively, and includes 2 lines of unchanged content around each changed | ||
| * section where possible. | ||
| * @param {string} before - The string representing the base for the comparison. | ||
| * @param {string} after - The string representing the changes being compared. | ||
| * @returns {string} The genereated text diff | ||
| */ | ||
| function generateDiff(before, after) { | ||
| const diffResult = diff.diffLines(before, after); | ||
| const penultimateDiffResult = diffResult[diffResult.length - 2] || {}; | ||
| // `diffLines` will always return at least one change object | ||
| const lastDiffResult = diffResult[diffResult.length - 1]; | ||
|
|
||
| // Add notice about newline at end of file | ||
| if (!lastDiffResult.value.endsWith('\n')) { | ||
| lastDiffResult.noNewline = true; | ||
| } | ||
| // If the last change is an addition and the penultimate change is a | ||
| // removal, then the last line of the file is also in the penultimate change. | ||
| // That's why we're checking to see if the newline notice is needed here as | ||
| // well. | ||
| if ( | ||
| lastDiffResult.added && | ||
| penultimateDiffResult.removed && | ||
| !penultimateDiffResult.value.endsWith('\n') | ||
| ) { | ||
| penultimateDiffResult.noNewline = true; | ||
| } | ||
|
|
||
| const diffLines = diffResult.flatMap( | ||
| ({ added, noNewline, removed, value }, index) => { | ||
| const lines = getTrimmedLines(value); | ||
| const changedLines = []; | ||
| if (added || removed) { | ||
| // Add up to 2 lines of context before each change | ||
| const previousContext = diffResult[index - 1]; | ||
| if ( | ||
| previousContext && | ||
| !previousContext.added && | ||
| !previousContext.removed | ||
| ) { | ||
| // The diff result prior to an unchanged result is guaranteed to be | ||
| // either an addition or a removal | ||
| const previousChange = diffResult[index - 2]; | ||
| const hasPreviousChange = previousChange !== undefined; | ||
| const previousContextLines = getTrimmedLines(previousContext.value); | ||
| // Avoid repeating context that has already been included in diff | ||
| if (!hasPreviousChange || previousContextLines.length >= 3) { | ||
| const linesOfContext = | ||
| hasPreviousChange && previousContextLines.length === 3 ? 1 : 2; | ||
| const previousTwoLines = previousContextLines | ||
| .slice(-1 * linesOfContext) | ||
| .map((line) => ` ${line}`); | ||
| changedLines.push(...previousTwoLines); | ||
| } | ||
| } | ||
| changedLines.push( | ||
| ...lines.map((line) => `${added ? '+' : '-'}${line}`), | ||
| ); | ||
| } else if (index > 0) { | ||
| // Add up to 2 lines of context following a change | ||
| const nextTwoLines = lines.slice(0, 2).map((line) => ` ${line}`); | ||
| changedLines.push(...nextTwoLines); | ||
| } | ||
| if (noNewline) { | ||
| changedLines.push('\\ No newline at end of file'); | ||
| } | ||
| return changedLines; | ||
| }, | ||
| ); | ||
| return diffLines.join('\n'); | ||
| } | ||
|
|
||
| module.exports = { generateDiff }; |
Oops, something went wrong.