Skip to content

delete normalizeDoc() #15909

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.

Sign up for GitHub

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

Closed
seiyab wants to merge 24 commits into from
Closed

delete normalizeDoc() #15909

Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0d86645
remove normalizeDoc()
seiyab Jan 10, 2024
2ca7b24
refactor
seiyab Jan 11, 2024
ae14b44
refactor
seiyab Jan 11, 2024
9f8cd1c
refactor
seiyab Jan 12, 2024
66f3c45
add test cases for flattenFill()
seiyab Jan 12, 2024
0f1a8f5
refactor
seiyab Jan 12, 2024
4a9f12e
update comment
seiyab Jan 12, 2024
da075a3
add format test cases
seiyab Jan 12, 2024
fbc82d2
refactor
seiyab Jan 12, 2024
ba90875
add changelog
seiyab Jan 12, 2024
389586e
resolve changelog issue
seiyab Jan 12, 2024
514cb86
refactor
seiyab Jan 13, 2024
80dcdec
make changelog clear
seiyab Jan 13, 2024
ae219bc
prefer path.map()
seiyab Jan 23, 2024
1faabb6
revive a line deleted unintentionally
seiyab Jan 23, 2024
3508c40
correct error message
seiyab Jan 23, 2024
0dd9963
simplify flattenFill
seiyab Jan 24, 2024
b06f367
Simplify
fisker Jan 24, 2024
84a7349
Simplify logic
fisker Jan 24, 2024
26aefe4
fix type issue and care edge case
seiyab Jan 25, 2024
e85797f
remove type
seiyab Jan 25, 2024
fabc5de
simplify logic
seiyab Jan 25, 2024
ed96c32
Merge branch 'main' into 15877-delete-normalizeDoc
seiyab Jan 26, 2024
5dd5f8a
update snapshots
seiyab Jan 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
remove type
  • Loading branch information
@seiyab
seiyab committed Jan 25, 2024
commit e85797f53a2dc10b078510bbdb444534e7ab95a4
11 changes: 4 additions & 7 deletions src/language-markdown/print-paragraph.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { fill } from "../document/builders.js";
import { DOC_TYPE_ARRAY, DOC_TYPE_FILL } from "../document/constants.js";
import { getDocParts, getDocType } from "../document/utils.js";
import { getDocType } from "../document/utils.js";

/**
* @typedef {import("../common/ast-path.js").default} AstPath
Expand Down Expand Up @@ -31,21 +31,18 @@ function flattenFill(docs) {
/** @type {Doc[]} */
const parts = [""];

/**
* @param {Doc[]} docArray
*/
(function rec(docArray) {
(function rec(/** @type {*} */ docArray) {
for (const doc of docArray) {
const docType = getDocType(doc);
if (docType === DOC_TYPE_ARRAY) {
rec(getDocParts(doc));
rec(doc);
continue;
}

let head = doc;
let rest = [];
if (docType === DOC_TYPE_FILL) {
[head, ...rest] = getDocParts(doc);
[head, ...rest] = doc.parts;
Copy link
Copy Markdown
Member

@fisker fisker Jan 24, 2024

Choose a reason for hiding this comment

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

I'm not sure about this part, do we need go deeper? head and docs in rest can also be DOC_TYPE_ARRAY or DOC_TYPE_FILL.

Copy link
Copy Markdown
Member

@fisker fisker Jan 24, 2024

Choose a reason for hiding this comment

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

I'm going to run some test first, prettier/prettier-regression-testing#435 (comment)

Copy link
Copy Markdown
Collaborator Author

@seiyab seiyab Jan 25, 2024

Choose a reason for hiding this comment

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

I'm not very sure it. But probably fill() isn't nested in paragraph. If we want to go deeper, we should take care that we should go deeper only even (odd for doc.parts) index element.

Copy link
Copy Markdown
Collaborator Author

@seiyab seiyab Jan 25, 2024

Choose a reason for hiding this comment

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

Searching "fill" in src/language-markdown, I confirmed fill() can't be nested in markdown. There are three use of fill():

  • printSentence() in print-sentence.js: sentence only contains word and whitespace that turns into simple docs like string or line-like. It won't have fill() as children.
  • if (shouldRemainTheSameContent(path)) { block in printer-markdown.js: same as printSentence().
  • flattenFill() in print-paragraph.js: paragraph can't be nested and the other children neither yield nested as described above.

So I think we don't need go deeper for elements of fill() as of now. But feel free to ask me to add recursive flattening for fill if you still worry about it.

if (rest.length % 2 === 1) {
rest.push("");
}
Expand Down