Use the system filename separator more consistently.

[kcarnold]

I normally use macOS, but tried using Windows today and quarto render failed. There are several problems but this was the first one:

ERROR: TypeError: Cannot convert undefined or null to object

 

Stack trace:
    at Function.keys (<anonymous>)
    at findIndexFile (file:///C:/Users/ka37/Documents/quarto-cli/src/project/types/website/website-sidebar-auto.ts:311:17)
    at nodesToEntries (file:///C:/Users/ka37/Documents/quarto-cli/src/project/types/website/website-sidebar-auto.ts:254:21)
    at sidebarItemsFromAuto (file:///C:/Users/ka37/Documents/quarto-cli/src/project/types/website/website-sidebar-auto.ts:139:16)    
    at expandAutoSidebarItems (file:///C:/Users/ka37/Documents/quarto-cli/src/project/types/website/website-sidebar-auto.ts:37:31)   
    at expandAutoSidebarItems (file:///C:/Users/ka37/Documents/quarto-cli/src/project/types/website/website-sidebar-auto.ts:65:31)   
    at async resolveSidebarItems (file:///C:/Users/ka37/Documents/quarto-cli/src/project/types/website/website-navigation.ts:948:24) 
    at async sidebarEjsData (file:///C:/Users/ka37/Documents/quarto-cli/src/project/types/website/website-navigation.ts:930:3)       
    at async sidebarsEjsData (file:///C:/Users/ka37/Documents/quarto-cli/src/project/types/website/website-navigation.ts:900:22)  

Notice that even though this is Windows, the stack trace reports paths in posix-style format. There's a lot of hard-coded forward-slashes in various parts of the quarto code, so I'd suggest that rather than trying to get everything right, just instead follow Deno's lead here and use URL semantics for everything.

My full setup is big, but something like this in _quarto.yml should suffice to reproduce:

website:
  sidebar:
    contents:
      - section: "Notes"
        icon: "pencil"
        contents: "notes/*"

(ideally I'd leave it that way, but on Windows I had to change that to notes\\*; that might be a separate bug report)

[cscheid]

Thanks for the PR! That's definitely a bug.

I think we might need a better fix, unfortunately. Your suggestion makes it so that we accurately use OS-local separators. However, I suspect that part of the reason we had / there to begin with is that we want documents authored on windows to render correctly in Linux (most commonly in CI deployments).

I believe your change makes it so that quarto would use \ on windows, but that's not the full story; we would prefer to normalize our representation to '/' and then resolve that correctly when in different OSs.

[cscheid]

See https://github.com/denoland/deno_std/blob/main/path/windows/mod.ts#L25, https://github.com/denoland/deno_std/blob/main/path/win32.ts#L7, and https://github.com/denoland/deno_std/blob/main/path/mod.ts#L47

[kcarnold]

I totally agree on normalizing to / instead.

[kcarnold]

My previous comment was left too quickly. I should add: that's the first thing I thought of, but I was scared off by how many times join is used in the codebase. Replacing the path import with path/posix might handle most of those all at once, but the simple find-replace encountered a bunch of errors (since https://github.com/denoland/deno_std/blob/main/path/mod.ts doesn't just dispatch to win32 or posix, so there were missing symbols).

Perhaps it would be better to think of paths as relative URLs and use that stdlib code instead, though I'd be a bit concerned this might break for projects on Windows network shares (UNC paths are kinda like URLs but not exactly).

[cscheid]

... and now you understand why this isn't a simple fix!

[cscheid]

This needs a more substantial rewrite. I appreciate the PR but I don't think we can salvage the approach.