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

Add commit offsets patch #27

Merged
merged 15 commits into from
Aug 26, 2024
Merged

Add commit offsets patch #27

merged 15 commits into from
Aug 26, 2024

Conversation

shaokeyibb
Copy link
Collaborator

Commit offsets patch 用于在源文档发生更改时动态修正 offset,该功能的算法大致如下:

  1. 检测文档更改,分为插入(insert)、删除(delete)、替换(replace)三种更改模式
  2. 对于这三种模式,分别对文档段落(diff.len)和文档段落之间的空行(diff.diff)计算位置
  3. 对上述两种更改,按照“diff 完全包含 offset”、“offset 完全包含 diff”、“diff 左半边在 offset 内,右半边在 offset 外”、“diff 右半边在 offset 内,左半边在 offset 外”四种方案进行位置修正
  4. 特殊的,引入 pseudoOffsetsDiffArr.beforeEverything 作为所有 offset 前的伪占位符来辅助计算(不需要 afterEverything,因为 所有 offset 后发生的更改不会影响这些 offset 的相对位置)
  5. 特殊的,对于跨段 replace,无法计算哪些部分属于哪些段落,因此直接讲这部分评论全部删除掉

该 pr 还同时支持了 path 更改,用于在迁移页面路径时使用。

cloudflare-workers/src/administration.ts Outdated Show resolved Hide resolved
cloudflare-workers/src/administration.ts Outdated Show resolved Hide resolved
cloudflare-workers/src/types.ts Show resolved Hide resolved
cloudflare-workers/src/administration.ts Outdated Show resolved Hide resolved
cloudflare-workers/src/administration.ts Outdated Show resolved Hide resolved
cloudflare-workers/src/administration.ts Outdated Show resolved Hide resolved
cloudflare-workers/src/db.ts Outdated Show resolved Hide resolved
cloudflare-workers/src/index.ts Show resolved Hide resolved
@Enter-tainer
Copy link
Member

记得有空resolve一下review的时候提出的comment~

@Enter-tainer
Copy link
Member

对于这三种模式,分别对文档段落(diff.len)和文档段落之间的空行(diff.diff)计算位置

我比较疑惑的是为什么这里需要有段落的概念?感觉直接对着所有 comment range 操作就可以了?

cloudflare-workers/src/administration.ts Outdated Show resolved Hide resolved
cloudflare-workers/src/administration.ts Outdated Show resolved Hide resolved
cloudflare-workers/src/db.ts Show resolved Hide resolved
@Enter-tainer
Copy link
Member

有一个事情我还不太清楚,假如一个文件里面有两个 insert 操作,那么第二个insert操作的offset是基于原文件的吗,还是基于第一个insert操作修改之后的文件的?

@Enter-tainer
Copy link
Member

Enter-tainer commented Aug 21, 2024

脑补了一会,好像把所有comment区间按照右端点排序之后,或许可以用差分/或者其他数据结构来维护一些东西,但是我没有仔细想,另外感觉del的处理可能还挺麻烦的。所以我觉得现在可能就直接n * m的暴力就好。。

@shaokeyibb
Copy link
Collaborator Author

有一个事情我还不太清楚,假如一个文件里面有两个 insert 操作,那么第二个insert操作的offset是基于原文件的吗,还是基于第一个insert操作修改之后的文件的?

基于原文件

@shaokeyibb
Copy link
Collaborator Author

脑补了一会,好像把所有comment区间按照右端点排序之后,或许可以用差分/或者其他数据结构来维护一些东西,但是我没有仔细想,另外感觉del的处理可能还挺麻烦的。所以我觉得现在可能就直接n * m的暴力就好。。

目前 diff 确实是差分的模式(comment2.start = len1+diff1, comment2.end = len1+diff1+len2

@Enter-tainer
Copy link
Member

脑补了一会,好像把所有comment区间按照右端点排序之后,或许可以用差分/或者其他数据结构来维护一些东西,但是我没有仔细想,另外感觉del的处理可能还挺麻烦的。所以我觉得现在可能就直接n * m的暴力就好。。

目前 diff 确实是差分的模式(comment2.start = len1+diff1, comment2.end = len1+diff1+len2

看起来现在这里确实有一些科技,要不您有空弄一个文档(在hackmd上就行)来demo一下这里的design?或者我们约一个语音的session来聊一下这里的东西

@Enter-tainer
Copy link
Member

a01ff42 我糊了一版我的那个思路,FYI

@shaokeyibb
Copy link
Collaborator Author

改好啦,可以看看

Copy link
Member

@Enter-tainer Enter-tainer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

记得setup一下测试~

cloudflare-workers/src/administration.ts Outdated Show resolved Hide resolved
cloudflare-workers/src/db.ts Outdated Show resolved Hide resolved
cloudflare-workers/src/administration.ts Outdated Show resolved Hide resolved
Copy link
Member

@Enter-tainer Enter-tainer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,我晚上再最后仔细过一次,没问题就merge🚀

Copy link
Member

@Enter-tainer Enter-tainer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

cloudflare-workers/src/utils.ts Outdated Show resolved Hide resolved
cloudflare-workers/src/types.ts Show resolved Hide resolved
@shaokeyibb shaokeyibb merged commit 48c5ffa into master Aug 26, 2024
5 checks passed
@shaokeyibb shaokeyibb deleted the hikarilan branch August 26, 2024 13:39
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