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

fix(markdown): incorrect list indentation alignment in markdown parser #4884

Conversation

hexh250786313
Copy link
Contributor

fix #4882

@fannheyward
Copy link
Member

This breaks src/__tests__/markdown/index.test.ts.

@hexh250786313
Copy link
Contributor Author

尝试重构了下 listItem 的渲染部分,不好改,先搁置

@fannheyward fannheyward marked this pull request as draft March 5, 2024 02:24
@fannheyward
Copy link
Member

尝试重构了下 listItem 的渲染部分,不好改,先搁置

Converted the PR to Draft.

@hexh250786313
Copy link
Contributor Author

hexh250786313 commented Mar 19, 2024

(Sorry for using chinese. It is hard to descript my question by my poor English)

export function bulletPointLine(indent, line) {
return isPointedLine(line, indent) ? line : toSpaces(BULLET_POINT) + line
}

@fannheyward issue 提到的这个问题的核心原因在于上面的函数对某些行多次调用,导致重复插入了多余的空格。而 marked 遍历 list item 是每次从最里层遍历整个层级一直遍历到最外层,这样会导致有些行会被 bulletPointLine 重复处理,举个例子:

- foo
  - hello
    - me
      you
      them

遍历结果:
第一次结果提取出第三层也就是 ['me', 'you', 'them'],排除 'me',因为 'me' 是有 - 标记的行,然后把 ['you', 'them'] 分别经过 bulletPointLine 处理,添加空格
第二次结果提取出第二层也就是 ['hello', 'me', 'you', 'them'],排除 'hello''me',然后把剩余的放入 bulletPointLine 进行处理
第三次也类似,最后 'you''them' 这两个没有标记 - 的行会再次被添加空格

简单来说就是在遍历的过程中,那些没有 - 标记的行会不断经过 bulletPointLine 添加空格,导致层级越深,这些行就越往右边缩进。原因就是上面所述的

而正确的处理应该是对于没有 - 标记的行,只需要被执行一次 bulletPointLine,添加一次空格就可以了

那么这个问题的关键就在于如何区分哪些行是已经添加过空格的,这里我有两个方案

  • 一个方案是给添加过空格的行增加一个标记,然后下次再次遍历到的时候就忽略这些行,最后再统一删除这些标记,这个方案的问题在于 markedrenderer 似乎没有提供 api 对最终的文本进行处理,也就是不能在 Renderer 单独处理,只能在调用的地方再处理一遍,或者对 marked 的调用再封装一个函数来处理
  • 另一个方案就是把插入的空格替换为 \u00A0,也就是不换行空格,然后遍历的时候判断有没有 \u00A0 就行,不过这个方案的弊端也很明显,假如 md 中确实存在不换行空格而且在 list item 中的,那么就有 bug,但是这种情况体感应该是概率很低的,所以也可以作为一种方案

还有一种就是把处理过的行存起来,然后遍历的时候,再通过对比进行判断,但是这个方案需要考虑的东西太多,例如如何正确判断当前行是已经处理过的行,如果正好有一样的文本怎么办,所以我个人认为这种也不可取

第一个方案是理论最可行的,但是他的实现并不优雅

这个问题不是什么大问题,不影响功能,只是个展示样式的错位,不过要处理起来也感觉也并不简单。有空研究下提供下意见

@fannheyward
Copy link
Member

对于没有 - 标记的行,只需要被执行一次 bulletPointLine,添加一次空格就可以

这个不对吧,比如 you 这个,就是要增加两次空格:

  1. 第三层处理的时候,增加一次空格
  2. 第二层,hello 这一层,也要增加一次空格,如果不增加,是不是就变成下面了?
- hello
  - me
  you

@hexh250786313
Copy link
Contributor Author

hexh250786313 commented Mar 19, 2024

@fannheyward 他这个函数是用来处理对齐 - 的,是为了保持当前层级和有 - 标记的行(也就是层级的第一行)保持对齐,而整个层级的缩进已经在 marked 的其他部分处理过了

假设 renderer 的 list item 的 - 标记 BULLET_POINT 设置为 *** (共 4 个字符)

*** hello
  *** me
  ~~~~              计算出 list item 层级的 BULLET_POINT 字符数为 4
      you
  ~~~~              bulletPointLine 给每一个当前层级没有 BULLET_POINT 标记的行添加 4 格缩进

所以这个地方不是处理整个层级缩进的,只处理层级自身的缩进对齐

@fannheyward fannheyward marked this pull request as ready for review March 19, 2024 11:55
Copy link

codecov bot commented Mar 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.58%. Comparing base (0d66554) to head (83c6d2f).

❗ Current head 83c6d2f differs from pull request most recent head e949328. Consider uploading reports for the commit e949328 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4884      +/-   ##
==========================================
+ Coverage   98.55%   98.58%   +0.02%     
==========================================
  Files         273      273              
  Lines       26077    26088      +11     
  Branches     5392     5393       +1     
==========================================
+ Hits        25701    25718      +17     
+ Misses        222      216       -6     
  Partials      154      154              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hexh250786313
Copy link
Contributor Author

按照第一个方案改了一个版本,缺点是需要在每一个 marked.setOptions 都要加上 hooks 来处理最终文本:

  marked.setOptions({
    renderer: new Renderer(),
    hooks: Renderer.hooks,
  })

@fannheyward
Copy link
Member

#4947 更新到了 marked 7,辛苦 rebase 测试一下

@hexh250786313 hexh250786313 force-pushed the fix/markdown_incorrect_list_indentation branch from 353b0d5 to 2a714a9 Compare March 20, 2024 11:39
@hexh250786313 hexh250786313 force-pushed the fix/markdown_incorrect_list_indentation branch from 2a714a9 to 83c6d2f Compare March 20, 2024 11:43
@hexh250786313
Copy link
Contributor Author

辛苦 rebase 测试一下

@fannheyward Done.

@fannheyward fannheyward merged commit 8c85d6f into neoclide:master Mar 20, 2024
2 of 3 checks passed
fannheyward added a commit that referenced this pull request Mar 26, 2024
9190bfe feat(list): add --buffer to list buffer diags (#4958)
fb030d4 fix(defaultAction) fallback to default action on emtpy expression (#4954)
846994b fix(inlayHint): more compact padding highlight group (#4950)
b01ae44 feat(semanticTokens)!: token highlight groups (#4667)
8c85d6f fix(markdown): incorrect list indentation alignment in markdown parser (#4884)
0d66554 fix(inlayHint): hl_mode = 'replace' for neovim (#4949)
1a8244e chore(package): update dependencies (#4948)
4f68722 feat(client): upgrade to marked 7 (#4947)
574c12c feat(package): @types/node@18 (#4946)
d299c66 feat(menu): C-j/k to navigate menu (#4945)
34d3d9d chore(deps): bump follow-redirects from 1.15.4 to 1.15.6 (#4944)
aead8f4 chore(CI): use Node 20 (#4940)
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.

Incorrect indentation alignment in markdown parser
2 participants