ionide / ionide-vscode-mechanic

Mechanic plugin for VS Code
https://github.com/fsprojects/Mechanic
MIT License
17 stars 4 forks source link

Fix #9: Choose project based on active file #16

Closed MangelMaxime closed 6 years ago

MangelMaxime commented 6 years ago

Careful, don't release this until we fixed #15 or we need a manual action on release.

MangelMaxime commented 6 years ago

Don't merge this PR yet, I should filter current file type.

forki commented 6 years ago

@Krzysztof-Cieslak when this is ready - please release ;-) I'm away from PC now

MangelMaxime commented 6 years ago

Should be ready for review.

@forki Perhaps, you could give us the action needed to release a new version to avoid deploying a broken version of mech.dll

forki commented 6 years ago

Ah good point. Easiest fix. Change the build script here to run dotnet publish in the src folder of mechanic.commandline. Just right before that line that copies the mech libs. And instead of copying them from debug folder, copy them from the /public folder within that debug folder. Or even better: do the above but for release mode.

Maxime Mangel notifications@github.com schrieb am Di., 13. März 2018, 17:50:

Should be ready for review.

@forki https://github.com/forki Perhaps, you could give us the action needed to release a new version to avoid deploying a broken version of mech.dll

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ionide/ionide-vscode-mechanic/pull/16#issuecomment-372736676, or mute the thread https://github.com/notifications/unsubscribe-auth/AADgNEMdk4XIOxSBYoiDQ6ZaM-asLtauks5td_jpgaJpZM4So84x .

MangelMaxime commented 6 years ago

I sent a PR https://github.com/fsprojects/Mechanic/pull/86

forki commented 6 years ago

Even better

forki commented 6 years ago

is this ready?

MangelMaxime commented 6 years ago

Yes, it's ready for review and test.

forki commented 6 years ago

I think we broke something

tried on mechanic itself:

image

MangelMaxime commented 6 years ago

Was the project loaded ? Does the file belongs to an fsproj ?

MangelMaxime commented 6 years ago

Seems like we were in this case:

https://github.com/ionide/ionide-vscode-mechanic/blob/98c8d1ad5c9b8548df1dfcc18321e0399251ccab/src/Mechanic.fs#L49-L54

In this case we probably need to execute:

https://github.com/ionide/ionide-vscode-mechanic/blob/98c8d1ad5c9b8548df1dfcc18321e0399251ccab/src/Mechanic.fs#L64-L85

forki commented 6 years ago

PR appreciated ;-)

MangelMaxime commented 6 years ago

Hum...

If the file is not associated to project file then running Mechanic is useless. We probably need to add the file to an fsproj first.

What do you think ?

forki commented 6 years ago

it is in the project.

The filter |> List.contains textEditor.document.uri.path dosesn't work.

I'm debugging it

forki commented 6 years ago

Ok I listed the files in the project and then the textEditor.document.uri.path:

image

obviously they are in different format. similar issue to https://github.com/ionide/ionide-vscode-mechanic/issues/18

do we already have a way to normilze that? /cc @Krzysztof-Cieslak

forki commented 6 years ago

how do we know if we are on windows or linux?

MangelMaxime commented 6 years ago

@forki Ah yes logic...

We can normalize the path: https://nodejs.org/api/path.html#path_path_normalize_path We can detect the OS:

open Fable.Import

if Node.Exports.os.platform() = Node.Base.NodeJS.Win32 then
    // We are under windows
Krzysztof-Cieslak commented 6 years ago

I think document.filename or document.uri.fsPath should have correct, normalized paths (from top of my head)

MangelMaxime commented 6 years ago

@forki Can I let you check that ? I don't have access to a Windows computer

forki commented 6 years ago

current solution works for me. maybe we should just keep it