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

Move the parser to a separate project #172

Merged
merged 3 commits into from
May 28, 2021
Merged

Conversation

sebastian
Copy link
Contributor

@sebastian sebastian commented May 28, 2021

The parser is the only module depending on fparsec. FParsec doesn't compile to JS using Fable because of it's C# dependencies... this was the simplest way I could find to quickly get our system compiling to JavaScript in order for us to make a test anonymizer desktop application based on reference.

There is one ugly aspect here which I would love some input on how to address, and that it that the parser types still live in the OpenDiffix.Core project. This is not particularly elegant... These types are used in our analyzer so need to be accessible by OpenDiffix.Core. I could move them to OpenDiffix.Parser, but that in turn would require OpenDiffix.Core to reference OpenDiffix.Parser, which again would mean I would end up pulling in the fparsec dependency, and the transpiling to JavaScript would then end up failing again... Another option would be to move the parser types to a shared "type project", but that seems unnecessarily complicated and just plain noisy.

The parser is the only module depending on fparsec. FParsec doesn't compile to JS using Fable because of it's C# dependencies... this was the simplest way I could find to quickly get our system compiling to JavaScript in order for us to make a test anonymizer desktop application based on reference.
@sebastian sebastian requested review from cristianberneanu and removed request for cristianberneanu May 28, 2021 09:49
@sebastian sebastian marked this pull request as draft May 28, 2021 09:52
We don't match on or use the result type anywhere. We are now embracing the exceptions instead ;)
@sebastian
Copy link
Contributor Author

This commit (81ebf46) is going to be a bit hard to review. Sorry.

The tests are the same in their functionality. As I moved the tests to the parser test project I no longer had access to the test helpers we had defined in the Core.Tests project. I didn't want to copy them over, nor did I want to move them to a separate tests help lib, so I simplified the tests instead by using the regular foo |> should equal bar pattern that we have been using everywhere. I should have done this in two steps to make the changes more visible, but well, failed to do so.

@sebastian sebastian marked this pull request as ready for review May 28, 2021 11:09
Copy link
Contributor

@cristianberneanu cristianberneanu left a comment

Choose a reason for hiding this comment

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

As long as we take this approach (Electron desktop app), I don't see a better way to do it.
But seeing the changes needed, I wonder if it wouldn't be simpler to build a regular dotnet GUI application instead. Or build the logic in the command line application and make the GUI a wrapper around that.

@sebastian
Copy link
Contributor Author

dotnet GUI application instead

I don't think there is a cross platform way of doing this at present? Well, there are a few, but they all seem super clunky?

@sebastian sebastian merged commit 3231db3 into master May 28, 2021
@sebastian sebastian deleted the sebastian/split-out-parser branch May 28, 2021 12:47
@cristianberneanu
Copy link
Contributor

I don't think there is a cross platform way of doing this at present?

My impression was that we are only targeting Windows for the GUI application.
The command line application could be the cross platform solution.

Well, there are a few, but they all seem super clunky?

Have you looked into Avalonia?

@sebastian
Copy link
Contributor Author

sebastian commented May 28, 2021

Have you looked into Avalonia?

Yes 😢 That's what I meant by clunky... but who knows, maybe it's not actually that bad in reality!
By clunky I partially mean clunky to write (but then probably all GUI frameworks are clunky to a certain extent), but also largely super clunky to use. I have played around with a number of their sample projects in the past, and didn't feel any of them were nice to use...

There is an F# version even: https://github.com/fsprojects/Avalonia.FuncUI


Alternatively there is also Uno but I like that even less.


That being said, it would be amazing to have a native environment, where we don't have to live in JavaScript.

@sebastian
Copy link
Contributor Author

sebastian commented May 28, 2021

You know, it could very well be that I am unfair against Avalonia, and that it's a much better fit than I think it is.
Do you want to spin up a super simple mini demo and see what it is like, @cristianberneanu?

@cristianberneanu
Copy link
Contributor

OK, I'll give a few of their samples a test drive and see how they feel like.

@cristianberneanu
Copy link
Contributor

I played a bit with this GUI library during the weekend and I wasn't left too amazed by it.

It is a bit clunky, as you said. I also encountered a few bugs (the scrollbar wasn't rendering properly in one case), the controls are not native, the documentation is lacking good examples, and they have a lot of issues unresolved on Github (over 1100).

It would be nice to keep all the code in dotnet and we might make it work for our simple use case, but we'll probably struggle with it.

@sebastian
Copy link
Contributor Author

and they have a lot of issues unresolved on Github (over 1100)

Ok, that's not a good start!

It would be nice to keep all the code in dotnet and we might make it work for our simple use case, but we'll probably struggle with it.

Would be great to have everything in dotnet for sure! Would save lots of hassle. I already had some dotnet -> js -> dotnet troubles in the very simple demo I created last week.

There are other Electron related alternatives like mentioned by @edongashi like Electron.NET. Unfortunately that doesn't seem very well maintained either. I played with the Electron.NET demo application last week too, but none of the IPC demos worked at all...

At least if we go the main electron route we have a stack that is very actively maintained.

@edongashi
Copy link
Member

The JS ecosystem is much better equipped for:

  • Data visualization
  • Syntax highlighting (for sql in the future)
  • Porting to a potential web version of the UI
    Etc...

@sebastian
Copy link
Contributor Author

Looking at the latest MAUI preview, maybe this ends up something we can use sooner rather than later after all? That is to become the officially sanctioned way of shipping dotnet GUI apps, and is cross platform to boot.

That being said: I have no experience with Maui (well, it doesn't really exist yet), nor Xamarin.Forms which it builds on top of... so cannot say anything about the developer experience, or the resulting user experiences once can develop. And F# support always lags behind severely, so we would maybe have to use C# for a while...

@sebastian
Copy link
Contributor Author

Fabulous is the slickest way (I have seen at least) of working with Xamarin.Forms from F#. It uses a Model-View-Update style architecture (like Elm) on top of Xamarin.Forms. It is also "made by Microsoft" (by some definition of that phrase)...

Regarding Fabulous + Maui: very much work in progress, or rather "let's wait for Maui and see": fabulous-dev/Fabulous#747

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.

None yet

3 participants