Skip to content

Commit

Permalink
Fix #62 Allow the formatter to ignore multiline instructions
Browse files Browse the repository at this point in the history 
Signed-off-by: Remy Suen <[email protected]>
  • Loading branch information
@rcjsuen
rcjsuen committed Feb 27, 2021
1 parent 9d9f5d9 commit 09fd5a9
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 28 deletions.
17 changes: 17 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,23 @@
# Changelog
All notable changes to this project will be documented in this file.

## [Unreleased]
### Added
- a new `FormatterSettings` interface for defining `ignoreMultilineInstructions` to ignore instructions that span multiple lines ([#62](https://github.com/rcjsuen/dockerfile-utils/issues/62))
```
export interface FormatterSettings extends FormattingOptions {
/**
* Flag to indicate that instructions that span multiple lines
* should be ignored.
*/
ignoreMultilineInstructions?: boolean;
}
```

### Changed
- altered the signature of the `format(string, FormattingOptions)` function to `format(string, FormatterSettings)`, this is a non-breaking change as `FormatterSettings` extends `FormattingOptions` ([#62](https://github.com/rcjsuen/dockerfile-utils/issues/62))

## [0.2.0] - 2021-01-19
### Added
- support the `--chmod` flag for ADD instructions ([#85](https://github.com/rcjsuen/dockerfile-utils/issues/85))
Expand Down
58 changes: 36 additions & 22 deletions src/dockerFormatter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
'use strict';

import {
TextDocument, TextEdit, Position, Range, FormattingOptions,
TextDocument, TextEdit, Position, Range
} from 'vscode-languageserver-types';
import { DockerfileParser } from 'dockerfile-ast';
import { FormatterSettings } from './main';

export class DockerFormatter {

private getIndentation(formattingOptions?: FormattingOptions): string {
private getIndentation(formattingOptions?: FormatterSettings): string {
let indentation = "\t";
if (formattingOptions && formattingOptions.insertSpaces) {
indentation = "";
Expand Down Expand Up @@ -45,7 +46,7 @@ export class DockerFormatter {
}
}

public formatOnType(document: TextDocument, position: Position, ch: string, options: FormattingOptions): TextEdit[] {
public formatOnType(document: TextDocument, position: Position, ch: string, options: FormatterSettings): TextEdit[] {
const dockerfile = DockerfileParser.parse(document.getText());
// check that the inserted character is the escape character
if (dockerfile.getEscapeCharacter() === ch) {
Expand Down Expand Up @@ -77,23 +78,25 @@ export class DockerFormatter {
}
}

const lines = [position.line + 1];
const line = position.line + 1;
const indentedLines: boolean[] = [];
indentedLines[lines[0]] = true;
return this.formatLines(document, document.getText(), lines, indentedLines, options);
const skippedLines: boolean[] = [];
indentedLines[line] = true;
skippedLines[line] = true;
return this.formatLines(document, document.getText(), [line], indentedLines, skippedLines, options);
}
return [];
}

public formatRange(document: TextDocument, range: Range, options?: FormattingOptions): TextEdit[] {
public formatRange(document: TextDocument, range: Range, options?: FormatterSettings): TextEdit[] {
const lines: number[] = [];
for (let i = range.start.line; i <= range.end.line; i++) {
lines.push(i);
}
return this.format(document, lines, options);
}

public formatDocument(document: TextDocument, options?: FormattingOptions): TextEdit[] {
public formatDocument(document: TextDocument, options?: FormatterSettings): TextEdit[] {
const lines: number[] = [];
for (let i = 0; i < document.lineCount; i++) {
lines.push(i);
Expand All @@ -110,57 +113,68 @@ export class DockerFormatter {
* @param options the formatting options to use to perform the format
* @return the text edits to apply to format the lines of the document
*/
private format(document: TextDocument, lines: number[], options?: FormattingOptions): TextEdit[] {
private format(document: TextDocument, lines: number[], options?: FormatterSettings): TextEdit[] {
let content = document.getText();
let dockerfile = DockerfileParser.parse(content);
const indentedLines: boolean[] = [];
const skippedLines: boolean[] = [];
for (let i = 0; i < document.lineCount; i++) {
indentedLines[i] = false;
skippedLines[i] = false;
}
for (let instruction of dockerfile.getInstructions()) {
let range = instruction.getRange();
if (range.start.line !== range.end.line) {
for (let i = range.start.line + 1; i <= range.end.line; i++) {
skippedLines[i] = true;
}
}
indentedLines[range.start.line] = false;
for (let i = range.start.line + 1; i <= range.end.line; i++) {
indentedLines[i] = true;
}
}
return this.formatLines(document, content, lines, indentedLines, options);
return this.formatLines(document, content, lines, indentedLines, skippedLines, options);
}

private formatLines(document: TextDocument, content: string, lines: number[], indentedLines: boolean[], options?: FormattingOptions): TextEdit[] {
private formatLines(document: TextDocument, content: string, lines: number[], indentedLines: boolean[], skippedLines: boolean[], options?: FormatterSettings): TextEdit[] {
const indentation = this.getIndentation(options);
const edits: TextEdit[] = [];
lineCheck: for (let line of lines) {
let startOffset = document.offsetAt(Position.create(line, 0));
for (let i = startOffset; i < content.length; i++) {
switch (content.charAt(i)) {
lineCheck: for (let i = 0; i < lines.length; i++) {
if (options && options.ignoreMultilineInstructions && skippedLines[lines[i]]) {
continue;
}

let startOffset = document.offsetAt(Position.create(lines[i], 0));
for (let j = startOffset; j < content.length; j++) {
switch (content.charAt(j)) {
case ' ':
case '\t':
break;
case '\r':
case '\n':
if (i !== startOffset) {
if (j !== startOffset) {
// only whitespace on this line, trim it
let edit = TextEdit.del({
start: document.positionAt(startOffset),
end: document.positionAt(i)
end: document.positionAt(j)
});
edits.push(edit);
}
// process the next line
continue lineCheck;
default:
// found a line that should be indented
if (indentedLines[line]) {
const originalIndentation = document.getText().substring(startOffset, i);
if (indentedLines[lines[i]]) {
const originalIndentation = document.getText().substring(startOffset, j);
// change the indentation if it's not what we expect
if (originalIndentation !== indentation) {
const edit = this.createFormattingEdit(document, startOffset, i, indentedLines[line], indentation);
const edit = this.createFormattingEdit(document, startOffset, j, indentedLines[lines[i]], indentation);
edits.push(edit);
}
} else if (i !== startOffset) {
} else if (j !== startOffset) {
// non-whitespace character encountered, realign
const edit = this.createFormattingEdit(document, startOffset, i, indentedLines[line], indentation);
const edit = this.createFormattingEdit(document, startOffset, j, indentedLines[lines[i]], indentation);
edits.push(edit);
}
// process the next line
Expand Down
14 changes: 13 additions & 1 deletion src/main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ import { TextDocument, Diagnostic, TextEdit, FormattingOptions } from 'vscode-la
import { DockerFormatter } from './dockerFormatter';
import { Validator } from './dockerValidator';

/**
* Settings for configuring the behaviour of the formatter.
*/
export interface FormatterSettings extends FormattingOptions {

/**
* Flag to indicate that instructions that span multiple lines
* should be ignored.
*/
ignoreMultilineInstructions?: boolean;
}

/**
* Error codes that correspond to a given validation error. These
* values are exposed for the purpose of allowing clients to identify
Expand Down Expand Up @@ -181,7 +193,7 @@ export interface ValidatorSettings {
* Analyzes the Dockerfile contained within the given string and
* provides an array of TextEdits back for formatting the document.
*/
export function format(content: string, options: FormattingOptions): TextEdit[] {
export function format(content: string, options: FormatterSettings): TextEdit[] {
const document = TextDocument.create("", "", 0, content);
let formatter = new DockerFormatter();
return formatter.formatDocument(document, options);
Expand Down
94 changes: 89 additions & 5 deletions test/dockerFormatter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
import * as assert from "assert";

import {
TextEdit, TextDocument, Position, Range, FormattingOptions
TextEdit, TextDocument, Position, Range
} from 'vscode-languageserver-types';
import { format } from '../src/main';
import { format, FormatterSettings } from '../src/main';
import { DockerFormatter } from '../src/dockerFormatter';

let formatter = new DockerFormatter();
Expand All @@ -16,7 +16,7 @@ function createDocument(content: string): any {
return TextDocument.create("uri://host/Dockerfile.sample", "dockerfile", 1, content);
}

function formatDocument(document: TextDocument, options?: FormattingOptions): TextEdit[] {
function formatDocument(document: TextDocument, options?: FormatterSettings): TextEdit[] {
if (!options) {
options = {
insertSpaces: false,
Expand All @@ -26,7 +26,7 @@ function formatDocument(document: TextDocument, options?: FormattingOptions): Te
return format(document.getText(), options);
}

function formatRange(document: TextDocument, range: Range, options?: FormattingOptions): TextEdit[] {
function formatRange(document: TextDocument, range: Range, options?: FormatterSettings): TextEdit[] {
if (!options) {
options = {
insertSpaces: false,
Expand All @@ -36,7 +36,7 @@ function formatRange(document: TextDocument, range: Range, options?: FormattingO
return formatter.formatRange(document, range, options);
}

function formatOnType(document: TextDocument, position: Position, ch: string, options?: FormattingOptions): TextEdit[] {
function formatOnType(document: TextDocument, position: Position, ch: string, options?: FormatterSettings): TextEdit[] {
if (!options) {
options = {
insertSpaces: false,
Expand Down Expand Up @@ -247,6 +247,42 @@ describe("Dockerfile formatter", function() {
assert.equal(edits.length, 0);
});
});

describe("ignore multiline", () => {
it("standard multiline with \\ ignores second line", () => {
const document = createDocument("RUN ls \\\nls");
const edits = formatDocument(document, { insertSpaces: false, tabSize: 4, ignoreMultilineInstructions: true });
assert.equal(edits.length, 0);
});

it("standard multiline with \\ ignores second line with backtick parser directive", () => {
const document = createDocument("#escape=`\nRUN ls `\nls");
const edits = formatDocument(document, { insertSpaces: false, tabSize: 4, ignoreMultilineInstructions: true });
assert.equal(edits.length, 0);
});

it("multiline with \\ formats first line", () => {
const document = createDocument(" RUN ls \\\nls");
const edits = formatDocument(document, { insertSpaces: false, tabSize: 4, ignoreMultilineInstructions: true });
assert.equal(edits.length, 1);
assert.equal(edits[0].newText, "");
assert.equal(edits[0].range.start.line, 0);
assert.equal(edits[0].range.start.character, 0);
assert.equal(edits[0].range.end.line, 0);
assert.equal(edits[0].range.end.character, 2);
});

it("multiline with \\ formats first line with backtick parser directive", () => {
const document = createDocument("#escape=`\n RUN ls \\\nls");
const edits = formatDocument(document, { insertSpaces: false, tabSize: 4, ignoreMultilineInstructions: true });
assert.equal(edits.length, 1);
assert.equal(edits[0].newText, "");
assert.equal(edits[0].range.start.line, 1);
assert.equal(edits[0].range.start.character, 0);
assert.equal(edits[0].range.end.line, 1);
assert.equal(edits[0].range.end.character, 2);
});
});
});

describe("range", function() {
Expand Down Expand Up @@ -469,6 +505,42 @@ describe("Dockerfile formatter", function() {
assert.equal(edits[0].range.end.line, 1);
assert.equal(edits[0].range.end.character, 0);
});

describe("ignore multiline", () => {
it("standard multiline with \\ ignores second line", () => {
const document = createDocument("RUN ls \\\nls");
const edits = formatRange(document, Range.create(0, 4, 1, 1), { insertSpaces: false, tabSize: 4, ignoreMultilineInstructions: true });
assert.equal(edits.length, 0);
});

it("standard multiline with \\ ignores second line with backtick parser directive", () => {
const document = createDocument("#escape=`\nRUN ls `\nls");
const edits = formatRange(document, Range.create(1, 4, 2, 1), { insertSpaces: false, tabSize: 4, ignoreMultilineInstructions: true });
assert.equal(edits.length, 0);
});

it("multiline with \\ formats first line", () => {
const document = createDocument(" RUN ls \\\nls");
const edits = formatRange(document, Range.create(0, 0, 1, 1), { insertSpaces: false, tabSize: 4, ignoreMultilineInstructions: true });
assert.equal(edits.length, 1);
assert.equal(edits[0].newText, "");
assert.equal(edits[0].range.start.line, 0);
assert.equal(edits[0].range.start.character, 0);
assert.equal(edits[0].range.end.line, 0);
assert.equal(edits[0].range.end.character, 2);
});

it("multiline with \\ formats first line with backtick parser directive", () => {
const document = createDocument("#escape=`\n RUN ls \\\nls");
const edits = formatRange(document, Range.create(1, 0, 2, 1), { insertSpaces: false, tabSize: 4, ignoreMultilineInstructions: true });
assert.equal(edits.length, 1);
assert.equal(edits[0].newText, "");
assert.equal(edits[0].range.start.line, 1);
assert.equal(edits[0].range.start.character, 0);
assert.equal(edits[0].range.end.line, 1);
assert.equal(edits[0].range.end.character, 2);
});
});
});

describe("multi line selection", function() {
Expand Down Expand Up @@ -865,6 +937,12 @@ describe("Dockerfile formatter", function() {
let edits = formatOnType(document, Position.create(0, 8), '\\');
assert.equal(edits.length, 0);
});

it("ignore multiline", () => {
const document = createDocument("RUN ls \nls");
const edits = formatOnType(document, Position.create(0, 7), '\\', { insertSpaces: false, tabSize: 4, ignoreMultilineInstructions: true });
assert.equal(edits.length, 0);
});
});

describe("backtick", function() {
Expand All @@ -884,6 +962,12 @@ describe("Dockerfile formatter", function() {
assert.equal(edits[0].range.end.line, 2);
assert.equal(edits[0].range.end.character, 0);
});

it("ignore multiline", () => {
const document = createDocument("#escape=`\nRUN ls \nls");
const edits = formatOnType(document, Position.create(1, 7), '`', { insertSpaces: false, tabSize: 4, ignoreMultilineInstructions: true });
assert.equal(edits.length, 0);
});
});
});
});

0 comments on commit 09fd5a9

Please sign in to comment.