Skip to content

refactor(IO): clean up ffAppendFDBuffer - #671

Merged
CarterLi merged 1 commit into
fastfetch-cli:devfrom
apocelipes:refactor-clean-up-io
Dec 22, 2023
Merged

refactor(IO): clean up ffAppendFDBuffer#671
CarterLi merged 1 commit into
fastfetch-cli:devfrom
apocelipes:refactor-clean-up-io

Conversation

@apocelipes

Copy link
Copy Markdown
Contributor

for 2 reasons:

  • Simplify the Code.
  • ffAppendFDBuffer can't make assumptions about how the read data will be processed, so the trim calls are inappropriate. All callers currently handle whitespace and newlines at the end of files themselves. So these trims are also redundant and can simply be removed.

for 2 reasons:

- Simplify the Code.
- ffAppendFDBuffer can't make assumptions about how the read data will
  be processed, so the trim calls are inappropriate.
@apocelipes

Copy link
Copy Markdown
Contributor Author

Comment out that ffStrbufEnsureFixedLengthFree will take care of NUL.

@CarterLi
CarterLi merged commit e85a4b9 into fastfetch-cli:dev Dec 22, 2023
@CarterLi

Copy link
Copy Markdown
Member

I think you are correct. Thanks

@CarterLi

Copy link
Copy Markdown
Member

image
image

Maybe need some fixes @apocelipes

CarterLi added a commit that referenced this pull request Dec 22, 2023
@CarterLi

Copy link
Copy Markdown
Member

It turns out that there are so many trailing line endings in sysfs. I fixed some of them, but surely not all. Although the change is right, it can be too dangerous.

@apocelipes

Copy link
Copy Markdown
Contributor Author

Yes. There are 40+ calls need to be checked. Maybe we should bring trims back and add a todo: "remove this after all check finished".

@apocelipes

Copy link
Copy Markdown
Contributor Author

@CarterLi I need to apologize for not checking the code of some modules carefully. Now I did a full code review and created a new PR: #673 . Please take a look.

@apocelipes
apocelipes deleted the refactor-clean-up-io branch December 25, 2023 00:38
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