From ebd3c2bcf7dd3d89607af01a59acd775c14fa683 Mon Sep 17 00:00:00 2001 From: Carlos Scheidegger Date: Tue, 24 Sep 2024 09:15:17 -0700 Subject: [PATCH 1/2] scss analyzer - fix more issues with brittle upstream parser --- src/core/sass.ts | 9 +- src/core/sass/analyzer/parse.ts | 14 +- .../docs/smoke-all/2024/09/24/issue-10870.qmd | 7 + tests/docs/smoke-all/2024/09/24/theme.scss | 166 ++++++++++++++++++ 4 files changed, 189 insertions(+), 7 deletions(-) create mode 100644 tests/docs/smoke-all/2024/09/24/issue-10870.qmd create mode 100644 tests/docs/smoke-all/2024/09/24/theme.scss diff --git a/src/core/sass.ts b/src/core/sass.ts index df3b67a56c..e5afb02b70 100644 --- a/src/core/sass.ts +++ b/src/core/sass.ts @@ -142,12 +142,11 @@ export async function compileSass( try { scssInput += "\n" + cssVarsBlock(scssInput); } catch (e) { - console.error("Error adding css vars block", e); - Deno.writeTextFileSync("scss-error.scss", scssInput); - console.error( - "This is a Quarto bug.\nPlease consider reporting it at https://github.com/quarto-dev/quarto-cli,\nalong with the scss-error.scss file that can be found in the current working directory.", + console.warn("Error adding css vars block", e); + Deno.writeTextFileSync("_quarto_internal_scss_error.scss", scssInput); + console.warn( + "This is a Quarto bug.\nPlease consider reporting it at https://github.com/quarto-dev/quarto-cli,\nalong with the _quarto_internal_scss_error.scss file that can be found in the current working directory.", ); - throw e; } // Compile the scss diff --git a/src/core/sass/analyzer/parse.ts b/src/core/sass/analyzer/parse.ts index c52aed39b9..901dbb2b74 100644 --- a/src/core/sass/analyzer/parse.ts +++ b/src/core/sass/analyzer/parse.ts @@ -14,10 +14,20 @@ export const makeParserModule = ( // it also doesn't like some valid ways to do '@import url' contents = contents.replaceAll("@import url", "//@import url"); - // it also really doesn't like statemnts that don't end in a semicolon + // it also really doesn't like statements that don't end in a semicolon // so, in case you are reading this code to understand why the parser is failing, // ensure that your SCSS has semicolons at the end of every statement. - // + // we try to work around this by adding semicolons at the end of declarations that don't have them + contents = contents.replaceAll( + /^(?!\/\/)(.*[^}/\s\n;])([\s\n]*)}(\n|$)/mg, + "$1;$2}$3", + ); + // It also doesn't like values that follow a colon directly without a space + contents = contents.replaceAll( + /(^\s*[A-Za-z0-9-]+):([^ \n])/mg, + "$1: $2", + ); + // This is relatively painful, because unfortunately the error message of scss-parser // is not helpful. diff --git a/tests/docs/smoke-all/2024/09/24/issue-10870.qmd b/tests/docs/smoke-all/2024/09/24/issue-10870.qmd new file mode 100644 index 0000000000..319f6f8292 --- /dev/null +++ b/tests/docs/smoke-all/2024/09/24/issue-10870.qmd @@ -0,0 +1,7 @@ +--- +format: + revealjs: + theme: theme.scss +--- + +## Hello. \ No newline at end of file diff --git a/tests/docs/smoke-all/2024/09/24/theme.scss b/tests/docs/smoke-all/2024/09/24/theme.scss new file mode 100644 index 0000000000..98c1716a22 --- /dev/null +++ b/tests/docs/smoke-all/2024/09/24/theme.scss @@ -0,0 +1,166 @@ + + +/*--scss:defaults --*/ +$yellow: #ffc627 !default; +$gold: #ffc627 !default; +$red: #8c1d40 !default; +$maroon: #8c1d40 !default; +$orange: #ff7f32 !default; +$blue: #00a3e0 !default; +$green: #78be20 !default; +$grey: #747474 !default; + +// fonts +// $font-family-sans-serif: "Arial" !default; + +// main colors +$link-color: $maroon !default; + +// font sizes and margins +$presentation-font-size-root: 46px !default; +$presentation-h1-font-size: 2.3em !default; +$presentation-h2-font-size: 1.5em !default; +$presentation-h3-font-size: 1.2em !default; +$presentation-h4-font-size: 1.1em !default; +$presentation-heading-color: $maroon !default; + +// inline code +$code-color: $blue !default; + +/*-- scss:mixins --*/ + +// Generates the presentation background, can be overridden +// to return a background image or gradient +@mixin bodyBackground() { + background: $backgroundColor; +} + +/*-- scss:rules --*/ + +// Change text colors against dark slide backgrounds + +.reveal .title { + line-height: 100px; + margin-top: 0.5em; +} + +.reveal .subtitle { + background: $gold; + font-size: $presentation-h2-font-size; + font-weight: bold; + margin-bottom: 1.5em; +} + +.reveal .author { + font-size: $presentation-h3-font-size; + font-weight: bold; + color: $maroon; +} + +.reveal .institute { + font-size: $presentation-h4-font-size; +} + +.reveal .date { + font-family: $font-family-monospace; + font-size: 0.8em; +} + +// This is a sentinel value that renderers can use to determine +// whether the theme is dark or light +// @if (color.blackness($backgroundColor) > $code-block-theme-dark-threshhold) { +// /*! dark */ +// } @else { +// /*! light */ +// } + +/*--Custom classes --*/ +.alert { + background: #FFC627; +} + +.center { + text-align: center; + display: block; +} + +.large {font-size: 150%} + +.small {font-size: 80%} + +.gold {color: #FFC627} +.maroon {color: #8C1D40} +.green {color: #78BE20} +.blue {color: #00A3E0} +.orange {color: #FF7F32} +.grey {color: #747474} +.fade { + opacity: 0.5; +} + +/* Two-column layout */ +.left-column { + width: 20%; + height: 93%; + float: left; +} +.left-column h2, .left-column h3 { + color: var(--inverse-link-color); +} +.left-column h2:last-of-type, .left-column h3:last-child { + color: var(--inverse-text-color); +} +.right-column { + width: 75%; + float: right; +} +.pull-left { + float: left; + width: 48%; +} +.pull-right { + float: right; + width: 48%; +} +.pull-right + * { + clear: both; +} + +hr { + content: none; + display: block; + border: none; + background-color: $gold; + height: 0.1em; +} + +blockquote { + border-left: solid 5px $gold; + padding-left: 1em; +} + +.container{ + display: flex; +} +.col{ + flex:1; +} + +.reveal.reveal img +{ + border: none; + box-shadow: none; + max-height: calc(75vh); + margin: auto; + display: flex; +} + +.rotated { + transform: rotate(270deg) +} + +.reveal .slide figure>figcaption, .reveal .slide img.stretch+p.caption, .reveal .slide img.r-stretch+p.caption { + font-size: 0.7em; + text-align: center; + color: $grey; +} From 9dad28e2308a4f050853c3135b77a2fc6c2c2d0d Mon Sep 17 00:00:00 2001 From: Carlos Scheidegger Date: Tue, 24 Sep 2024 09:18:32 -0700 Subject: [PATCH 2/2] scss analyzer - only emit warning if compilation succeeds --- src/core/sass.ts | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/core/sass.ts b/src/core/sass.ts index e5afb02b70..290badd008 100644 --- a/src/core/sass.ts +++ b/src/core/sass.ts @@ -139,14 +139,14 @@ export async function compileSass( ...userRules, '// quarto-scss-analysis-annotation { "origin": null }', ].join("\n\n"); + let failed = false; + // deno-lint-ignore no-explicit-any + let e: any = null; try { scssInput += "\n" + cssVarsBlock(scssInput); - } catch (e) { - console.warn("Error adding css vars block", e); - Deno.writeTextFileSync("_quarto_internal_scss_error.scss", scssInput); - console.warn( - "This is a Quarto bug.\nPlease consider reporting it at https://github.com/quarto-dev/quarto-cli,\nalong with the _quarto_internal_scss_error.scss file that can be found in the current working directory.", - ); + } catch (_e) { + e = _e; + failed = true; } // Compile the scss @@ -157,6 +157,18 @@ export async function compileSass( minified, md5HashBytes(new TextEncoder().encode(scssInput)), ); + + if (failed) { + console.warn("Error adding css vars block", e); + console.warn( + "The resulting CSS file will not have SCSS color variables exported as CSS.", + ); + Deno.writeTextFileSync("_quarto_internal_scss_error.scss", scssInput); + console.warn( + "This is likely a Quarto bug.\nPlease consider reporting it at https://github.com/quarto-dev/quarto-cli,\nalong with the _quarto_internal_scss_error.scss file that can be found in the current working directory.", + ); + } + if (!Deno.env.get("QUARTO_SAVE_SCSS")) { return result; }