Skip to content

Conversation

@Numpsy
Copy link
Contributor

@Numpsy Numpsy commented May 16, 2025

refs ionide/ionide-vscode-fsharp#1708

As mentioned in ionide/ionide-vscode-fsharp#1708 (comment), when I try to load a solution filter for one of my projects in Ionide 7.25.8 I don't get any projects loaded for the solution.
(I do get the guid entries described in ionide/ionide-vscode-fsharp#2067, but that's a different issue).

So - I had a go at adding a new unit test for a solution / solution filter pair, and when I tried debugging that I found:

  1. When it read the path to the solution file out of the filter file, it only got the file name of the parent solution rather than the path (the solution and filter are in the same directory). It then failed to load that solution.
    So - try to fix that by getting the absolute path to the parent solution based on the path to the solution filter.

  2. When parseSln is called with a filter list, it calls makeAbsoluteFromSlnDir on each project in the parent solution when searching in the filter list. That failed to match anything if the paths in the filter are relative rather than absolute. Try to fix that by calling makeAbsoluteFromSlnDir on each project in the filter file, before loading the parent solution with them.

I tried dropping the modified ProjInfo dll into my Ionide install and that appeared to fix it for my solution, but I'm working all this out as I go along so I'm not sure if this is correct and/or complete approach.

@Numpsy
Copy link
Contributor Author

Numpsy commented May 16, 2025

Not sure if it needs fantomas running or not - it changed all sorts of things unrelated to this change when I ran the fake build

@baronfel
Copy link
Collaborator

I tried dropping the modified ProjInfo dll into my Ionide install

Yeah, that'll work :D Nice job maintaining binary compat so that that worked!

@Numpsy
Copy link
Contributor Author

Numpsy commented May 17, 2025

Not sure if it needs fantomas running or not - it changed all sorts of things unrelated to this change when I ran the fake build

It appears that Fantomas is failing to parse FsLibLog.fs, which makes the CheckFormat build step report that files need to be formatted but then fail in a way that doesn't fail the build step so the CI still succeeds. InspectSln.fs is already reported as needing formatting in any case

@dawedawe
Copy link

Not sure if it needs fantomas running or not - it changed all sorts of things unrelated to this change when I ran the fake build

It appears that Fantomas is failing to parse FsLibLog.fs, which makes the CheckFormat build step report that files need to be formatted but then fail in a way that doesn't fail the build step so the CI still succeeds. InspectSln.fs is already reported as needing formatting in any case

Ouch, that's some interesting parser behaviour.

let private knownProviders = [
#if !FABLE_COMPILER
(SerilogProvider.isAvailable, SerilogProvider.create)
(MicrosoftExtensionsLoggingProvider.isAvailable, MicrosoftExtensionsLoggingProvider.create)
#endif
]

Just deleting the ! in the if directive makes the parser fail. (Or commenting the whole directive) As fantomas tries all combinations it sees that failure. At least that's my current understanding.
The parser should handle the closing bracket in the same way, regardless of the directive, of course.

Copy link
Member

@TheAngryByrd TheAngryByrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

I tried dropping the modified ProjInfo dll into my Ionide install and that appeared to fix it for my solution, but I'm working all this out as I go along so I'm not sure if this is correct and/or complete approach.

I think this seems fine. Having more tests with examples is great and we can add more as people find them.

It appears that Fantomas is failing to parse FsLibLog.fs, which makes the CheckFormat build step report that files need to be formatted but then fail in a way that doesn't fail the build step so the CI still succeeds. InspectSln.fs is already reported as needing formatting in any case

I ended up fixing this in #232 so shouldn't be a problem anymore.

@Numpsy
Copy link
Contributor Author

Numpsy commented May 17, 2025

rebased on top of the formatting changes

refs ionide/ionide-vscode-fsharp#1708

1) When reading the path to the solution file out of the filter file, create the abolsute path to it based on the filter path, instead of just using the name directly (it might just be the solution file name, with on path)

2) When parseSln is called with a filter list, it calls makeAbsoluteFromSlnDir on each project in the parent solution when searching in the filter list. That fails to match anything in the paths in the filter are relative rather than absolute. Try to fix that by calling makeAbsoluteFromSlnDir on each project in the filter file.
@Numpsy Numpsy marked this pull request as ready for review May 17, 2025 21:06
@TheAngryByrd TheAngryByrd merged commit 131f0bb into ionide:main May 17, 2025
3 checks passed
@TheAngryByrd
Copy link
Member

TheAngryByrd commented May 17, 2025

Should be available once in Ionide 7.25.10 once it's finished publishing and the marketplace indexes the package.

@Numpsy Numpsy deleted the slnf_tweaks branch May 18, 2025 08:40
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.

4 participants