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

请求合入:忽略输出功能&代码改进&bug修复 #20

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jywhy6
Copy link

@jywhy6 jywhy6 commented Jul 3, 2022

添加当epub文件存在时忽略输出的功能;
使用Path.Combine代替硬编码的路径,使代码跨平台可用;
自动过滤xhtml中的非UTF-8字符,避免epub转换失败。

@jywhy6 jywhy6 changed the title 请求合入一项新功能和一项代码改进 请求合入:忽略输出功能&代码改进 Jul 3, 2022
@jywhy6 jywhy6 changed the title 请求合入:忽略输出功能&代码改进 请求合入:忽略输出功能&代码改进&bug Jul 4, 2022
@jywhy6 jywhy6 changed the title 请求合入:忽略输出功能&代码改进&bug 请求合入:忽略输出功能&代码改进&bug修复 Jul 4, 2022
@Aeroblast
Copy link
Owner

感谢贡献,过段时间测试一下合并。

粗看了一眼最后那个过滤,貌似有点问题。不过滤会出什么问题吗?如果能输出文件,感觉还是不过滤比较好。

代码功能不是“过滤 xhtml 中的非 UTF-8 字符”,那些字符是合法的。按照帖子的说法是“移除 well-formed XML 中不合法的字符”。

这不是很合适,因为亚马逊上架检查对格式的要求不算特别严格,但是 .NET 的 XML 官方库严得要死。如果有这种字符,粗暴地过滤掉相当于改变了内容。应该考虑让它变得合法,比如写成转义的形式。

@jywhy6
Copy link
Author

jywhy6 commented Jul 4, 2022

粗看了一眼最后那个过滤,貌似有点问题。不过滤会出什么问题吗?如果能输出文件,感觉还是不过滤比较好。

如果不过滤,XML解析会抛出异常,且无法通过设置reader属性解决,导致epub无法生成(文档是茶杯头的画集,里面有个控制字符0x1b)。

代码功能不是“过滤 xhtml 中的非 UTF-8 字符”,那些字符是合法的。按照帖子的说法是“移除 well-formed XML 中不合法的字符”。

提交描述我回家改一下。

这不是很合适,因为亚马逊上架检查对格式的要求不算特别严格,但是 .NET 的 XML 官方库严得要死。如果有这种字符,粗暴地过滤掉相当于改变了内容。应该考虑让它变得合法,比如写成转义的形式。

确实是改变了内容,所以会输出警告。我个人觉得改变内容比程序crash好一点。另外,KindleUnpack本身无此问题。

@Aeroblast
Copy link
Owner

那么还是建议试着写成转义,比如这个控制字符写成。感觉这样应该可以避免 XML 库报错。

这种意外混进来的控制字符删了就删了,但如果删掉 emoji 之类的有意义字符,警告归警告,补救起来是很麻烦的。

@jywhy6
Copy link
Author

jywhy6 commented Jul 4, 2022

那么还是建议试着写成转义,比如这个控制字符写成。感觉这样应该可以避免 XML 库报错。

这种意外混进来的控制字符删了就删了,但如果删掉 emoji 之类的有意义字符,警告归警告,补救起来是很麻烦的。

搞定了,已修改 commit,已确认最后输出到 epub 的字符和 KindleUnpack 一致。

@Aeroblast
Copy link
Owner

感谢,之后这边测试下合并。

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.

2 participants