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

Make the ICodeSaver.SaveCodeToFile method asynchronous #100

Open
SeanFarrow opened this issue Nov 28, 2022 · 7 comments
Open

Make the ICodeSaver.SaveCodeToFile method asynchronous #100

SeanFarrow opened this issue Nov 28, 2022 · 7 comments

Comments

@SeanFarrow
Copy link

it would be really nice if the ICodeSave.SaveCodeToFile method was made asynchronous. Or maybe we should add a new method and obsolete the old one?

@jeffward01
Copy link
Collaborator

I think this is a great idea. Here is the PR for it: #101

It will go out in the next release once it is merged 😄

Great suggestion @SeanFarrow !! Please keep the suggestions coming!


Side note: I have a bunch of code and fluent syntax builders that I want to push, I will soon! I have not had time yet.

Thanks again for the suggestion here!

@jeffward01
Copy link
Collaborator

jeffward01 commented Nov 28, 2022

I did notice this on it:

Let's say I have a directory that is like this /myDirectory/ with no files or sub-directories in it.

Then I pass in this path to CodeSaver: /myDirectory/someSubDirectory/myfile.cs

It will throw an error that it cannot find the path. This is Microsoft System.IO that throws the error, not us. Its just the nature of System.IO

What do you guys think about adding a check in there to ensure that it can write to the file? Or we can be more brutal and do something like this:

var fi = new FileInfo(path);
Directory.Create(fi.Directory.FullName);
// ~~~ continue to write the file

Thats a bit more invasive, but also will make peoples lives 'easier'. Maybe its better to do a check and throw an exception instead? What do you guys think @SeanFarrow @MilleBo

@SeanFarrow
Copy link
Author

SeanFarrow commented Nov 28, 2022 via email

@jeffward01
Copy link
Collaborator

@SeanFarrow - Done, I added the check on both SaveCodeToFileAsync and SaveCodeToFile

If you have any other ideas please feel free to share!

@SeanFarrow
Copy link
Author

@jeffward01 The only things I might do differently are:
use the FormatAsync method of formatter, passing in a CalcellationToken if possible.
See whether we can replace the use of FileStreams/StreamWriter with File.WriteTextAsync.

Other than that, this looks really good.

@jeffward01
Copy link
Collaborator

@SeanFarrow - no problem, I just implemented the SaveCodeToFileAsync as async because that was the one you had asked for.

I agree completely that we should implement async methods wherever possible. My personal preference is to implement both Sync and Async versions, because in my experience sometimes calling .GetAwaiter().GetResult(); is not the same as a Sync method.

Here is a good write-up about it:

The article above refrences this blog post, which is also a good write up:


I plan to sweep and implement async wherever possible, and always include the cancellationToken.

I saw in some microsoft documentation, that for public facing SDK's they were handling the cancellationToken like this:

public async Task SomeMethodAsync(string someParam, CancellationToken cancellationToken =  default)
{
  // This is the interesting part imo
  cancellationToken.ThrowIfCancellationRequested()

  // do work...

}

Thanks for the suggestion! when I have some time i'll implement more async methods. if there are any blockers for you, feel free to do the same and we'll get it pulled in!

@SeanFarrow
Copy link
Author

SeanFarrow commented Nov 28, 2022 via email

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

No branches or pull requests

2 participants