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(core): Fixed workspace setting does not append a newline to the end of a line in an existing index file #1875 #1876

Conversation

maroKanatani
Copy link
Contributor

@maroKanatani maroKanatani commented Feb 4, 2025

Status

READY

Description

Fixed workspace setting does not append a newline to the end of a line in an existing index file.

I don't know the details of the code, so if you think another way is better or whatever, you can close this PR.

Related PRs

List related PRs against other branches:

branch PR
other_pr_production link
other_pr_master link

Todos

  • Tests
  • Documentation
  • Changelog Entry (unreleased)

Steps to Test or Reproduce

Please follow the steps in the following Issue

@maroKanatani maroKanatani force-pushed the fix/workspace-end-of-line-increasing branch from 81c3a45 to 06fad3b Compare February 5, 2025 01:04
@maroKanatani maroKanatani force-pushed the fix/workspace-end-of-line-increasing branch from 06fad3b to 419fa48 Compare February 5, 2025 01:09
Copy link
Member

@soartec-lab soartec-lab left a comment

Choose a reason for hiding this comment

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

@maroKanatani
Thank you for the nice update. I made one change request and a question

Comment on lines 135 to 136
.map((imp) => `export * from '${imp}';\n`)
.join(),
Copy link
Member

Choose a reason for hiding this comment

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

It should be aligned with the pattern written in else.

await fs.outputFile(
indexFile,
uniq(imports)
.map((imp) => `export * from '${imp}';`)
.join('\n') + '\n',
);

Suggested change
.map((imp) => `export * from '${imp}';\n`)
.join(),
.map((imp) => `export * from '${imp}';`)
.join('\n'),

Copy link
Contributor Author

@maroKanatani maroKanatani Feb 9, 2025

Choose a reason for hiding this comment

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

I was a little worried that if I implemented it with .join('\n'), the last line would not have a newline.

"export * from './zod/pet/pet';\n" +
"export * from './zod/store/store';\n" +
"export * from './zod/user/user';\n" +
+"export * from './types';\n" // my pattern
-"export * from './types';"    // with `.join('\n')`

With .join('\n') + '\n' before the modification, a newline is inserted even if the result of uniq(imports) is empty.

Copy link
Member

Choose a reason for hiding this comment

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

I see. You're right.

@@ -132,8 +132,8 @@ export const writeSpecs = async (
await fs.appendFile(
indexFile,
uniq(importsNotDeclared)
.map((imp) => `export * from '${imp}';`)
.join('\n') + '\n',
.map((imp) => `export * from '${imp}';\n`)
Copy link
Member

Choose a reason for hiding this comment

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

If there is already a newline at the end, will a new definition be added below that newline?
In that case, you may need to first remove the original trailing newline and then add another trailing newline.

Copy link
Contributor Author

@maroKanatani maroKanatani Feb 9, 2025

Choose a reason for hiding this comment

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

If there is already a newline at the end, will a new definition be added below that newline?

No, the behavior appears to be the same, especially before this modification.

No, the behavior does not seem to change, especially before this modification.
I have confirmed that it is appended by preparing and executing petstore index.ts with a new line at the end as shown below.

before

export * from './zod/user/user';
export * from './types';
export * from './zod/store/store';
- export * from './zod/pet/pet'; // remove this line

after

export * from './zod/user/user';
export * from './types';
export * from './zod/store/store';
+ export * from './zod/pet/pet';

@soartec-lab soartec-lab self-assigned this Feb 8, 2025
@maroKanatani
Copy link
Contributor Author

@soartec-lab Thanks for the review! I have returned your comment.

Copy link
Member

@soartec-lab soartec-lab left a comment

Choose a reason for hiding this comment

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

Thanks!

@soartec-lab soartec-lab merged commit 727cbb5 into orval-labs:master Feb 10, 2025
@soartec-lab soartec-lab added the bug Something isn't working label Feb 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When I set workspace and generate code, the line breaks keep increasing.
3 participants