Remove some nullable disable comments #2312
Conversation
* FormatDetecot * ImageFormatManager * DecoderUtilities (Not needed)
|
After these only 10 files have the annotation |
| @@ -27,7 +27,7 @@ public static IImageEncoder DetectEncoder(this Image source, string filePath) | |||
| Guard.NotNull(filePath, nameof(filePath)); | |||
|
|
|||
| string ext = Path.GetExtension(filePath); | |||
| IImageFormat format = source.GetConfiguration().ImageFormatsManager.FindFormatByFileExtension(ext); | |||
| IImageFormat? format = source.GetConfiguration().ImageFormatsManager.FindFormatByFileExtension(ext); | |||
There was a problem hiding this comment.
Ah... This is one of the areas I want to have a look at. Ideally this method should be rewritten to use the try-out pattern.
e.g
bool ImageFormatsManager.TryFindFormatByFileExtension(ext, [NotNullWhen(true)] out IImageFormat? format)We'd return named tuples for async equivalents.
There was a problem hiding this comment.
Ok gonna have a look if I can easily rewrite that to this pattern
There was a problem hiding this comment.
Legend. Thanks! It's something I very much wish we'd done from the start.
JimBobSquarePants
added
API
codequality
breaking
src/ImageSharp/Image.FromBytes.cs
Outdated
| @@ -27,7 +26,7 @@ public static IImageFormat DetectFormat(ReadOnlySpan<byte> data) | |||
| /// <param name="data">The byte span containing encoded image data to read the header from.</param> | |||
| /// <exception cref="ArgumentNullException">The options are null.</exception> | |||
| /// <returns>The mime type or null if none found.</returns> | |||
| public static IImageFormat DetectFormat(DecoderOptions options, ReadOnlySpan<byte> data) | |||
| public static IImageFormat? DetectFormat(DecoderOptions options, ReadOnlySpan<byte> data) | |||
There was a problem hiding this comment.
We still need to convert this and above to use the Try...Out pattern
There was a problem hiding this comment.
Do you want to convert InternalDetectFormat from Image.Decode.cs to the Try Pattern or just return IImageFormat?
There was a problem hiding this comment.
Ideally it should use the Try pattern all the way down.
There was a problem hiding this comment.
And at this Point it gets tricky. In Order to do this I have to also touch 'DiscoverDecoder' this already has IImageFormat as out parameter. When trying to refactor this to the try pattern we are in a situation where one ore the other could be null :(
What do you think if we change the implementation here tho that a format must be passed to DiscoverDecoder?
There was a problem hiding this comment.
Having a second look and we wouldn't be able to adopt the Try pattern there actually. The method is used within a lambda which doesn't allow ref or out parameters..
src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessorHelpers.cs
Outdated
Show resolved
Hide resolved
Ok i have no clue how I searched. In total there a more nullable disable in the Solution.. Probably I searched while in a specific scope ... oO |
#2231
Removed the nullable disable in all places where fixing it was an easy task