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

Draft: Multibyte wb support #23430

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jukuisma
Copy link
Contributor

@jukuisma jukuisma commented Oct 6, 2024

  • Mark this if you consider it ready to merge
  • I've added tests (optional)
  • I wrote some lines in the book (optional)

Description

@jukuisma
Copy link
Contributor Author

jukuisma commented Oct 7, 2024

TODO:

  • Update pull request description and commit messages
  • Double check: Draft: Multibyte wb support #23430 (comment) and drop the comment
  • Add unit test cases for valid extra spaces and invalid chars
  • Check if the book needs updating
  • Fix help messages and man pages if needed

Comment on lines +2206 to +2208
// TODO: Do we need some bound check for
// `core->offset + block_offset`? Other
// functions don't seem to implement any.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO

libr/core/cmd_write.inc.c Outdated Show resolved Hide resolved
for (i = 0; i < uil; i++) {
c = input[i];
// Ignore whitespaces, \r and \n chars
if (c == ' ' || c == '\r' || c == '\n') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use isspace() its more portable and fast and comes with libc. Also handles tab

Copy link
Contributor Author

Choose a reason for hiding this comment

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

continue;
}
// Check that user input only contains ones and zeros
else if (c != '0' && c != '1') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for else after continue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

@trufae trufae left a comment

Choose a reason for hiding this comment

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

.

@jukuisma jukuisma force-pushed the feature-multibyte-wb branch 2 times, most recently from 5e9ade2 to a39784e Compare October 7, 2024 17:57
@trufae
Copy link
Collaborator

trufae commented Oct 7, 2024

Lgtm

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