Improvements/additions to Subtitle Line parsing #32
Conversation
Since I forgot it in my last PR >.>
| /// <returns>A SsaWrapStyle corresponding to the value parsed from the input string</returns> | ||
| public static SsaWrapStyle FromString(this string rawString) => | ||
| int.TryParse(rawString, out int rawInt) == false ? | ||
| SsaWrapStyle.None: // basically an arbitrary choice, could also throw an exception here instead |
There was a problem hiding this comment.
Just wanted to draw your attention to this, as I'm not sure what you'd prefer the library do when it fails to parse the value. If you'd rather have it throw an exception, I can change it.
There was a problem hiding this comment.
I'm also looking at this again and realizing the == false is redundant since it's a ternary operator instead of the if statement I originally had...
Will leave it unless you want it changed, or if I go in there anyways to change to throwing an exception.
There was a problem hiding this comment.
Indeed, the two options have their pros and cons.
You can leave it like this for the moment. I can't quite get cases where things could go wrong but if I have some in the future, I'll consider if the other option (throwing an exception) is more adapted.
|
Thanks for PR. |
Currently only SSA and SRT remove formatting/styles
(which are actually removed from beginning of the following line)
|
This should be good for you to take a look at and merge now. |
|
It's merged! Thanks for your contribution and our discussion. |
This PR will include several changes:
\nand/or\N, depending on the wrap style) into multiple items in theSubtitleItem.Lineslist.I wanted to open the PR now in case you don't want to include the last feature in the library. It'll likely be another couple days before I finish that, so if you do want to include it, maybe just drop a comment so I know I'm good to push it to the PR when I'm done. If you don't want to include it, go ahead and handle this PR right away.