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

not-prettier flaw #2621

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions build/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const VALID_FLAW_CHECKS = new Set([
"image_widths",
"bad_pre_tags",
"sectioning",
"not_prettier",
]);

// TODO (far future): Switch to "error" when number of flaws drops.
Expand Down
123 changes: 115 additions & 8 deletions build/flaws.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const path = require("path");

const chalk = require("chalk");
const got = require("got");
const prettier = require("prettier");
const FileType = require("file-type");
const imagemin = require("imagemin");
const imageminPngquant = require("imagemin-pngquant");
Expand Down Expand Up @@ -34,6 +35,13 @@ function injectFlaws(doc, $, options, { rawContent }) {
injectBadBCDQueriesFlaws(options.flawLevels.get("bad_bcd_queries"), doc, $);

injectPreTagFlaws(options.flawLevels.get("bad_pre_tags"), doc, $, rawContent);

injectNotPrettierFlaws(
options.flawLevels.get("not_prettier"),
doc,
$,
rawContent
);
}

function injectSectionFlaws(doc, flaws, options) {
Expand Down Expand Up @@ -267,14 +275,6 @@ function injectBadBCDQueriesFlaws(level, doc, $) {
function injectPreTagFlaws(level, doc, $, rawContent) {
if (level === FLAW_LEVELS.IGNORE) return;

function escapeHTML(s) {
return s
.replace(/&/g, "&")
.replace(/"/g, """)
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;");
}

// Over the years, we've accumulated a lot of Kuma-HTML where the <pre> tags
// are actually full of HTML. Almost exclusively we've observed <pre> tags whose
// content is the HTML produced by Prism in the browser. Instead, in these cases,
Expand Down Expand Up @@ -392,6 +392,113 @@ function injectPreTagFlaws(level, doc, $, rawContent) {
}
}

function escapeHTML(s) {
return s
.replace(/&/g, "&amp;")
.replace(/"/g, "&quot;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;");
}

function unescapeHTML(h) {
return h
.replace(/&amp;/g, "&")
.replace(/&quot;/g, '"')
.replace(/&lt;/g, "<")
.replace(/&gt;/g, ">");
}

function injectNotPrettierFlaws(level, doc, $, rawContent) {
if (level === FLAW_LEVELS.IGNORE) return;

const getNextId = () => `not_prettier${doc.flaws.not_prettier.length + 1}`;

$("div.hidden pre").remove(); // XXX shouldn't need this. In some other PR.

$("pre").each((i, pre) => {
if ($("code", pre).length) {
return;
}
const $pre = $(pre);
const className = $pre.attr("class") || "";
if (className.includes("noprettier")) {
// Been marked to deliberately not be touched
return;
}
const match = className.match(/brush:?\s*([\w_-]+)/i);
if (!match) {
return;
}
const codeBefore = $pre.text();
let codeAfter = "";
let errorString = "";
if (match[1] === "js") {
try {
codeAfter = prettier.format(codeBefore, {
semi: true,
parser: "babel",
});
} catch (error) {
errorString = error.toString();
}
} else if (match[1] === "css") {
try {
codeAfter = prettier.format(codeBefore, {
parser: "css",
});
} catch (error) {
errorString = error.toString();
}
}
if ((codeAfter && codeBefore.trim() !== codeAfter.trim()) || errorString) {
if (!("not_prettier" in doc.flaws)) {
doc.flaws.not_prettier = [];
}
const id = getNextId();
const flaw = {
id,
fixable: false,
before: codeBefore,
};
for (const { line, column } of findMatchesInText(
codeBefore,
unescapeHTML(rawContent)
)) {
flaw.line = line;
flaw.column = column;
flaw.fixable = true;
}

if (errorString) {
flaw.explanation = "Can't even be formatted with Prettier";
flaw.error = errorString;
} else {
// We have a flaw with some hope of fixing!
(flaw.explanation = "Prettier would like to disagree"),
(flaw.suggestion = codeAfter);
if (flaw.line && flaw.column) {
flaw.fixable = true;
}

// But always update it
$pre.text(codeAfter);
}
doc.flaws.not_prettier.push(flaw);
$pre.attr("data-flaw", id);
}
});

if (
level === FLAW_LEVELS.ERROR &&
doc.flaws.not_prettier &&
doc.flaws.not_prettier.length
) {
throw new Error(
`not_prettier flaws: ${doc.flaws.not_prettier.map(JSON.stringify)}`
);
}
}

async function fixFixableFlaws(doc, options, document) {
if (!options.fixFlaws || document.isArchive) return;

Expand Down
20 changes: 20 additions & 0 deletions client/src/document/toolbar/flaws.scss
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,24 @@
li.fixed_flaw {
text-decoration: line-through;
}

div.not_prettier_error {
p {
font-weight: bold;
margin-bottom: 1px;
}
pre {
background-color: #ffeef0;
border-color: #d0454c;
}
}
pre.not_prettier_diff {
background-color: #f5f5f5;
code {
background-color: #f5f5f5;
span {
color: #3e3e3e;
}
}
}
}
106 changes: 106 additions & 0 deletions client/src/document/toolbar/flaws.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
BadBCDQueryFlaw,
BadPreTagFlaw,
SectioningFlaw,
NotPrettierFlaw,
} from "../types";
import "./flaws.scss";

Expand Down Expand Up @@ -143,6 +144,8 @@ export function ToggleDocumentFlaws({
}))
.sort((a, b) => b.count - a.count);

const flawsCountsSum = flawsCounts.reduce((a, b) => a + b.count, 0);

return (
<div
id={FLAWS_HASH.slice(1)}
Expand Down Expand Up @@ -255,6 +258,14 @@ function Flaws({
images={doc.flaws.images}
/>
);
case "not_prettier":
return (
<NotPrettier
key="not_prettier"
sourceFolder={doc.source.folder}
flaws={doc.flaws.not_prettier}
/>
);
case "image_widths":
return (
<ImageWidths
Expand Down Expand Up @@ -884,3 +895,98 @@ function ImageWidths({
</div>
);
}

function NotPrettier({
sourceFolder,
flaws,
}: {
sourceFolder: string;
flaws: NotPrettierFlaw[];
}) {
// XXX rewrite to a hook
const [opening, setOpening] = React.useState<string | null>(null);
useEffect(() => {
let unsetOpeningTimer: ReturnType<typeof setTimeout>;
if (opening) {
unsetOpeningTimer = setTimeout(() => {
setOpening(null);
}, 3000);
}
return () => {
if (unsetOpeningTimer) {
clearTimeout(unsetOpeningTimer);
}
};
}, [opening]);

const filepath = sourceFolder + "/index.html";

function openInEditor(key: string, line?: number, column?: number) {
const sp = new URLSearchParams();
sp.set("filepath", filepath);
if (line) sp.set("line", `${line}`);
if (column) sp.set("column", `${column}`);
console.log(
`Going to try to open ${filepath}:${line}:${column} in your editor`
);
setOpening(key);
fetch(`/_open?${sp.toString()}`).catch((err) => {
console.warn(`Error trying to _open?${sp.toString()}:`, err);
});
}

const { focus } = useAnnotations(flaws);

return (
<div className="flaw flaw__not_prettier">
<h3>{humanizeFlawName("not_prettier")}</h3>

{flaws.map((flaw, i) => {
const key = flaw.id;
return (
<details key={key}>
<summary>
<code>{flaw.explanation}</code>{" "}
<a
href={`file://${filepath}`}
onClick={(event: React.MouseEvent) => {
event.preventDefault();
openInEditor(key, flaw.line, flaw.column);
}}
>
line{" "}
{flaw.line && flaw.column
? `${flaw.line}:${flaw.column}`
: "line unknown"}
</a>{" "}
{opening && opening === key && <small>Opening...</small>}{" "}
<span
role="img"
aria-label="Click to highlight"
title="Click to highlight"
style={{ cursor: "zoom-in" }}
onClick={() => {
focus(key);
}}
>
👀
</span>{" "}
{flaw.fixable && <FixableFlawBadge />}{" "}
</summary>
{flaw.error && (
<div className="not_prettier_error">
<p>Failed to execute Prettier on the code</p>
<pre>{flaw.error}</pre>
</div>
)}
{flaw.before && flaw.suggestion && (
<pre className="not_prettier_diff">
<ShowDiff before={flaw.before} after={flaw.suggestion} />
</pre>
)}
</details>
);
})}
</div>
);
}
8 changes: 8 additions & 0 deletions client/src/document/types.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ export interface BadPreTagFlaw extends GenericFlaw {
type: BadPreTagType;
}

export interface NotPrettierFlaw extends GenericFlaw {
before: string;
error?: string;
line?: number;
column?: number;
}

export interface MacroErrorMessage extends GenericFlaw {
name: string;
error: {
Expand All @@ -76,6 +83,7 @@ type Flaws = {
images: ImageReferenceFlaw[];
bad_pre_tags: BadPreTagFlaw[];
sectioning: SectioningFlaw[];
not_prettier: NotPrettierFlaw[];
image_widths: ImageWidthFlaw[];
};

Expand Down