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

Fix watch issue on Windows #107

Merged
merged 1 commit into from
Mar 21, 2023

Conversation

bdukes
Copy link
Contributor

@bdukes bdukes commented Mar 3, 2023

When running elm-review --watch on Windows, the first review works successfully, but after a change is made to a file, the following error occurs:

-- ELM-REVIEW ERROR ----------------------------------------------- GLOBAL ERROR

: Found several modules named `Views.Example.Main`

I found several modules with the name `Views.Example.Main`. Depending on how
I choose to resolve this, I might give you different reports. Since this is a
compiler error anyway, I require this problem to be solved. Please fix this then
try running `elm-review` again.

Here are the paths to some of the files that share a module name:
- Website/DesktopModules/MVC/Example/Views/Example/Main.elm
- Website\DesktopModules\MVC\Example\Views\Example\Main.elm

It is possible that you requested me to look at several projects, and that
modules from each project share the same name. I don't recommend reviewing
several projects at the same time, as I can only handle one `elm.json`.

It appears that the first run uses file paths that have been made OS-agnostic (i.e. which uses / instead of \), but the path from the watcher is not made OS-agnostic, so it creates a situation where elm-review thinks the same file is at two different paths.

The initial loading of files in elm-files.js calls OsHelpers.makePathOsAgnostic on the path.

return {
path: OsHelpers.makePathOsAgnostic(relativeFilePath),
source,
lastUpdatedTime,
ast: cachedAst || (await readAst(options, elmParserPath, source))
};

This function matches the removeWindowsSeparators function in watch.js, so I replaced its usages, as well as adding usages that were missing.

function makePathOsAgnostic(path_) {
return path_.replace(/.:/, '').replace(/\\/g, '/');
}

function removeWindowsSeparators(string) {
return string.replace(/.:/, '').replace(/\\/g, '/');
}

@jfmengels
Copy link
Owner

Hey @bdukes!
Sorry for the delay in looking at your PR, I completely missed it!
I can't test this on Windows, but if it works for you then I'm good with it, since the changes look good.

Thank your for looking into this! ❤️

@jfmengels jfmengels merged commit a8d37e7 into jfmengels:master Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants