Skip to content

Conversation

@KevinRansom
Copy link
Contributor

While working on the packagemanager a bit. I felt the need to clean up some of the code around directives.

Most especially this:

            | IHash (ParsedHashDirective(c,arg,_),_) -> 
                fsiConsoleOutput.uprintfn "%s" (FSIstrings.SR.fsiInvalidDirective(c, String.concat " " arg))  // REVIEW: uprintnfnn - like other directives above
                istate,Completed None  (* REVIEW: cont = CompletedWithReportedError *)

If the #directive is invalid, we just printout a message in white and then move on.

E.g.
image

Note, that the code before and after the definition is still compiled and executed.

I decided to turn it into a warning, so that existing scripts that relied on us not failing would still continue to work.

image

@KevinRansom KevinRansom requested review from brettfo and cartermp June 30, 2020 01:09
@cartermp cartermp changed the title cleanup fsi a bit Warn on invalided FSI directives. Jun 30, 2020
Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

Good improvement!

@brettfo
Copy link
Member

brettfo commented Jun 30, 2020

Minor suggestion: copy over the localized strings, since we know they're correct.

@KevinRansom KevinRansom merged commit 31a9ba7 into dotnet:master Jul 6, 2020
@KevinRansom KevinRansom deleted the directivescleanup branch August 19, 2020 20:22
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
* cleanup fsi a bit

* Update baselines
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.

3 participants