Skip to content

Conversation

@Mikachu2333
Copy link
Collaborator

问题描述

  1. 许多关键函数没有描述
  2. N/A

方案与实现

  1. 按照 Doxygen注释语法 增加了注释
  2. 统一换行行数等小问题

其它

本pr应当被最后合并

@github-actions
Copy link

Hi @Mikachu2333,

❤️ 感谢你的贡献!你的 PR 当前基于 main 分支,请修改使用 dev 分支

@Mikachu2333 Mikachu2333 changed the base branch from main to dev August 17, 2025 07:14
@ccmywish
Copy link
Contributor

按照 Doxygen注释语法 增加了注释

之前试过 Doxygen,但是生成的文档效果太差,索性不用了。但是注释一定程度上遵循它的语法还是可以的

但是我们不会完全遵循它,比如,我们用了根本不是 Doxygen 支持的符号,如 @consult,是因为这里用这个词很合适,已经完全不管 Doxygen 如何了。


统一换行行数等小问题

每个函数之间空两行吧,src/framework/core.c 基本是这么做的。

如果遇到一堆函数和一堆函数有明显的划分,就用三个空行划分。

这一点,我加到代码格式文档里

@Mikachu2333
Copy link
Collaborator Author

我遇到了一个问题,xy_parent_dir 这个函数,在win上执行的效果不是很对,你认为这应该是正确的结果吗?

如果正确,我就加个warning,不正确我就顺手修了然后在assert里面加两条

say(xy_normalize_path ("~/"));
say(xy_parent_dir (xy_normalize_path ("~/")));
C:\Users\Admin\
C:\Users\Admin

@ccmywish
Copy link
Contributor

https://github.com/RubyMetric/chsrc/blob/main/tool/git-ignore-vscode-settings.ps1

我在 tool 目录中加了一个文件,你直接运行这个文件就行了,就不会再干扰 .vscode/settings 了。

.gitignore 那个方法应该没用,因为它已经被 git add 过了。

@ccmywish
Copy link
Contributor

say(xy_normalize_path ("~/"));
say(xy_parent_dir (xy_normalize_path ("~/")));
C:\Users\Admin\
C:\Users\Admin

这个结果是对的

  1. 第一个:因为 ~/ 本身就带了一个 /,所以结果肯定是 C:\Users\Admin\
  2. 第二个:xy_parent_dir() 返回的确实应该就是一个目录名本身,目录名本身是不带 \

这里其实体现了命名的精妙,xy_normalize_path() 是一种 path,它返回的包含了 \,这依然是一种 path

@Mikachu2333

This comment was marked as outdated.

@ccmywish
Copy link
Contributor

say(xy_normalize_path ("~/"));
say(xy_parent_dir (xy_normalize_path ("~/")));

你看下代码格式那个 .md 文件,我刚好写了这种函数嵌套的风格,应该写成:

say (xy_normalize_path("~/"));
say (xy_parent_dir (xy_normalize_path("~/")));

函数嵌套时,里面的函数到底有没有一个空格,我觉得可以随意一些,但是外部函数和 ( 之间留一个空格肯定清晰一些

@ccmywish
Copy link
Contributor

是vscode一个设置的锅,把该设置禁用了就ok了现在

哪个设置?我这边也会不经意触发(我无法复现),我感觉是我的某个插件带来的

@Mikachu2333

This comment was marked as resolved.

@Mikachu2333

This comment was marked as resolved.

@ccmywish
Copy link
Contributor

"C_Cpp.autoAddFileAssociations": false,

这个设置不错,你可以提交进去

但是我的应该不止是 C_Cpp.autoAddFileAssociations 带来的影响,因为它总是加进去类似 css 相关的东西

@Mikachu2333
Copy link
Collaborator Author

1. 第一个:因为 `~/` 本身就带了一个 `/`,所以结果肯定是 `C:\Users\Admin\`

2. 第二个:`xy_parent_dir()` 返回的确实应该就是一个目录名本身,目录名本身是不带 `\` 的

这里其实体现了命名的精妙,xy_normalize_path() 是一种 path,它返回的包含了 \,这依然是一种 path

说实话,第二条我不太认同,如果它只是将一个路径处理为目录,应该直接叫xy_to_dir之类的东西,但是既然加了parent,我认为该函数有责任将传入的路径进一步处理为父目录,无论它的格式有多奇怪(在我看来这与提前的错误处理有相似之处)

要是你坚持这么认为那我就给parent加个warning,因为这不是很符合我的习惯(也和通常情况下的预期结果不符,例如python的路径处理模块、rust的pathbuf模块等)

@Mikachu2333

This comment was marked as off-topic.

@Mikachu2333
Copy link
Collaborator Author

还有,xy_normalize_path也很奇怪,在win上传入 ~ 竟然返回的是 . 而不是类似 C:\\users的东西……

@Mikachu2333
Copy link
Collaborator Author

为啥 br和hr用了不同的方式定义呢,明明这俩作用相近,结果既不是在同一个地方定义的,实行方式也不相同……

想把hr塞到br旁边,一起用函数的形式定义了

void br () { puts (""); }
#define hr() say ("--------------------------------");

@ccmywish
Copy link
Contributor

还有,xy_normalize_path也很奇怪,在win上传入 ~ 竟然返回的是 . 而不是类似 C:\users的东西……

这里确实不应该,你可以看一下实现,只处理了 ~/,根本没有处理 ~

我们的实现都是“按需实现”,项目需要现在实现这个功能了,就添加上,还没遇到的,就暂时先不管。既然提出来了,就刚好是个机会修改。

@ccmywish
Copy link
Contributor

如果涉及到多个问题,一定要拆分成多个 PR 提交,尽管麻烦一点,但是我能很快合并,不用因为其他某些小问题导致重要的东西迟迟合并不进来

@ccmywish
Copy link
Contributor

为啥 br和hr用了不同的方式定义呢,明明这俩作用相近,结果既不是在同一个地方定义的,实行方式也不相同……
想把hr塞到br旁边,一起用函数的形式定义了

br() 是极其简单的,没有任何变化。

但是 hr() 里到底应该有多少个 - 是没有定义的,是参考 chsrc 的输出才决定 - 到底应该多少个合适,所以塞到了 chsrc 框架中。

@ccmywish
Copy link
Contributor

举个例子,我这里也有css和html相关的扩展,但是我没出现,我看看能不能试着顺手排除一下

我没办法复现,总是编辑着编辑着突然出现,下次出现的时候我留意一下

@ccmywish
Copy link
Contributor

说实话,第二条我不太认同,如果它只是将一个路径处理为目录,应该直接叫xy_to_dir之类的东西,但是既然加了parent,我认为该函数有责任将传入的路径进一步处理为父目录,无论它的格式有多奇怪(在我看来这与提前的错误处理有相似之处)

要是你坚持这么认为那我就给parent加个warning,因为这不是很符合我的习惯(也和通常情况下的预期结果不符,例如python的路径处理模块、rust的pathbuf模块等)

额,不好意思,我搞错了,我以为重点是对比有无 \,压根忽视了参数是 ~/

say (xy_normalize_path("~/"));
say (xy_parent_dir (xy_normalize_path("~/")));

正确的结果应该是:

C:\Users\Admin\
C:\Users

@Mikachu2333
Copy link
Collaborator Author

想为模板的这里增加注释但是没看懂干啥的

图片

@ccmywish
Copy link
Contributor

是否定义了 _getsrc() _setsrc() _resetsrc()

@Mikachu2333
Copy link
Collaborator Author

这里也是
图片

@ccmywish
Copy link
Contributor

这个PR不是 增加大量函数描述 吗??怎么出现了如此大的格式化,搞来搞去又回到格式化问题上去了。

我建议开始讨论:

#264

@Mikachu2333
Copy link
Collaborator Author

草,我的错我的错,格式化函数空行的时候习惯性的顺手又把那个空格问题处理了……,回退一下我

@Mikachu2333 Mikachu2333 force-pushed the feat/improve_comments branch from 4281f32 to edc84ee Compare August 17, 2025 12:00
@Mikachu2333 Mikachu2333 marked this pull request as ready for review August 17, 2025 12:17
@Mikachu2333 Mikachu2333 force-pushed the feat/improve_comments branch from 0836f3e to 6c9ead6 Compare August 18, 2025 11:21
@ccmywish
Copy link
Contributor

ccmywish commented Aug 18, 2025

我建议这个 PR 应该关闭,又是大量的不同主题的更改合并到一个 PR,又包含了许多格式化.

PR 应该这样:

  1. 一个 PR 解决一个具体的问题
  2. 暂时不接受任何任何任何形式的格式化的 PR
  3. 一个问题如果覆盖范围太大,应该首先提一个 issue,看是否有必要,不要提交覆盖范围过大的 PR,不但合并很难,review 起来也很难
  4. 一个PR应该解决碰到什么问题解决什么问题,还没碰到的不要预先解决(比如,你执意想要格式化一些代码,也只能格式化你正在修改的函数部分的代码,不能格式化任何其他部分,否则一旦开始,这个PR又开始泛滥性修改)

小PR的好处是极其明显的,如果你的PR足够小,我早已经可以合并非常多内容了,比如 .vscode/settings.json 的修改。

我看了下通过正则进行格式化的依然不行,有一些注释,比如 streql() 是直接写在注释里的,streql() 专门写在一起是为了表示这个函数。

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