MonoS / SupMover

SupMover - Shift timings and Screen Area of PGS/Sup subtitle
GNU Affero General Public License v3.0
39 stars 4 forks source link

`--trace` option & New parsing with optional output file argument #18

Closed YourMJK closed 4 months ago

YourMJK commented 4 months ago

See discussion: https://github.com/MonoS/SupMover/pull/17#issuecomment-1924849735

MonoS commented 4 months ago

Seems fine to me, I'll test it a bit tomorrow and let you know :)

YourMJK commented 4 months ago

@MonoS Did you come around to testing this? Let me know if you want me to change something.

MonoS commented 4 months ago

Did you come around to testing this?

Yeah, i've put some comment in your code, i can see them in this PR before your latest comment.

YourMJK commented 4 months ago

Oh, don't know why but I can't see those unfortunately ¯\(ツ)

MonoS commented 4 months ago

Oh, don't know why but I can't see those unfortunately ¯_(ツ)_/¯

That's probably because I don't know how to use github :)

Anyway:

In any case I can see the code review from this link https://github.com/MonoS/SupMover/pull/18/files/8968b7ce51e5384b9684d1a867d82b3ef87393cb And looks like this image

YourMJK commented 4 months ago

That's probably because I don't know how to use github :)

That's fine, I'm not familiar with code review on GitHub either :) But I think you need to click on "Review changes" and then "Submit review" for me to see it. Those comments are still pending (see your screenshot).

  • The new --trace doesn't have the check for its option without dash

That was on purpose since it's part of the old syntax and thus doesn't need to be backwards compatible. But for consistency I can of course add that check as well.

Agreed on the other two points!

YourMJK commented 4 months ago

Should be fixed now

MonoS commented 4 months ago

Merged, thank you :)