Skip to content

fix: Use posix paths in import statements in Sass strings #1684

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Sep 13, 2024

Conversation

gadenbuie
Copy link
Collaborator

@gadenbuie gadenbuie commented Sep 12, 2024

Fixes #1681

We are currently relying on os.path.join() and os.path.realpath() which is usually the right choice, but because we're putting the path into another string, we need better escaping for path separators (or better path separators).

The new approach is to use Path(path).as_posix() to convert c:\mydir\...\_01_functions.scss to c:/mydir/.../_01_functions.scss, which then drops into the Sass strings easily.

The code below was run on Windows 11, where the problem is reproducible and highlights the core issue.

from shiny.ui._theme import path_pkg_preset
import sass
import os
from pathlib import Path

path_functions = path_pkg_preset("shiny", "_01_functions.scss")
path_functions
#> 'C:\\Users\\garrick\\Documents\\external\\shiny_css_compilation_demo\\.venv\\Lib\\site-packages\\shiny\\www\\shared\\sass\\preset\\shiny\\_01_functions.scss'
f'@import "{path_functions}"'
#> '@import "C:\\Users\\garrick\\Documents\\external\\shiny_css_compilation_demo\\.venv\\Lib\\site-packages\\shiny\\www\\shared\\sass\\preset\\shiny\\_01_functions.scss"'
f'@import "{Path(path_functions).as_posix()}"'
#> '@import "C:/Users/garrick/Documents/external/shiny_css_compilation_demo/.venv/Lib/site-packages/shiny/www/shared/sass/preset/shiny/_01_functions.scss"'

@cpsievert
Copy link
Collaborator

Should we be doing the same for include_paths? Or is the double-quoting not an issue there?

@gadenbuie
Copy link
Collaborator Author

gadenbuie commented Sep 12, 2024

Should we be doing the same for include_paths? Or is the double-quoting not an issue there?

No, those paths are passed to sass.compile() and are use for import resolution. As in the paths are handled entirely in Python. The weirdness happens when we take the path and put it in the string literal containing the Sass code.

@gadenbuie gadenbuie merged commit 695f9cb into main Sep 13, 2024
48 checks passed
@gadenbuie gadenbuie deleted the fix/1681-escape-windows-path-sep branch September 13, 2024 13:19
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants