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

自定义表情设置添加详细的路径信息输出 #996

Merged

Conversation

nicocatxzc
Copy link
Contributor

自定义表情功能添加输出详细实际读取路径
方便不同用户在不同的环境下可能存在差异的情况下自行排除错误

@kanodaisuki
Copy link
Contributor

#995 (comment)

信息应该在这个函数里面输出。

function update_custom_smilies_list()

@nicocatxzc
Copy link
Contributor Author

#995 (comment)

信息应该在这个函数里面输出。

function update_custom_smilies_list()

通过测试发现代码放在这部分不会输出路径,而放在get_custom_smilies_list中才可以正常读取路径的变化并输出

@KotoriK
Copy link
Collaborator

KotoriK commented Nov 27, 2024

get_custom_smilies_list()在多个地方调用,这样的写法是否会导致不该出现调试消息的地方输出了调试消息?建议还是减少这类函数的副作用

@nicocatxzc
Copy link
Contributor Author

get_custom_smilies_list()在多个地方调用,这样的写法是否会导致不该出现调试消息的地方输出了调试消息?建议还是减少这类函数的副作用

感谢指正!
根据您的顾虑,我加强了一下判断逻辑,调试信息仅会在管理页调用时才会输出

表情面板输出:push_custom_smilies()
除了前面两个更新逻辑,只有此处使用了get_custom_smilies_list(),

只有comments评论区生成会调用这个函数,而在is_admin的限制下应该不会误将调试信息输出到前端了

@kanodaisuki
Copy link
Contributor

感谢指正! 根据您的顾虑,我加强了一下判断逻辑,调试信息仅会在管理页调用时才会输出

表情面板输出:push_custom_smilies() 除了前面两个更新逻辑,只有此处使用了get_custom_smilies_list(),

只有comments评论区生成会调用这个函数,而在is_admin的限制下应该不会误将调试信息输出到前端了

这个做法实在是令人眼前一黑,不夸张地说。

正如 @KotoriK 所说,get_custom_smilies_list()在多个地方调用,函数也已经明确定义返回值为Array,不应该在里面进行任何输出。至于is_admin()限制,更像是掩耳盗铃,难道你想在自己登录的情况下打开评论区都出现这些文字?

我的建议是,再好好思考一下这几个函数的作用,尤其是update_custom_smilies_list(),你说放这个函数里面没有输出,思考一下会不会是触发方式不对。

另外,既然都输出路径了,你不如更进一步优化一下,把表情包图片输出给用户预览,示意系统都加载了哪些表情包。

加油。

image

@nicocatxzc
Copy link
Contributor Author

已迁移调试信息到update_custom_smilies_list()中,
也许是wordpress清洗了不应该出现的调试信息,但我不应该忽视这样简单粗暴地处理问题可能导致的风险。

已按照建议添加了收录的表情包图片预览

@KotoriK
Copy link
Collaborator

KotoriK commented Nov 28, 2024

LGTM
虽然感觉能做成REST API端点会更好,但现在这样也提不出什么问题

@mirai-mamori mirai-mamori merged commit dea5a82 into mirai-mamori:Preview-but-no-bugs Nov 30, 2024
@nicocatxzc nicocatxzc deleted the detailed_debug_info branch December 8, 2024 14:38
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.

4 participants