Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Update detab.c (Fixed #43) #90

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

Open
berkenvr wants to merge 1 commit into ohkimur:main
base: main
Choose a base branch
Loading
from berkenvr:fix-exercise-1-20

Conversation

@berkenvr
Copy link

@berkenvr berkenvr commented Aug 19, 2025
edited
Loading

The incorrect solution for exercise 1.20 has been corrected.
For detailed discussion, please refer to the issue #43.

@berkenvr berkenvr changed the title (削除) Fixed #43 (削除ここまで) (追記) Update detab.c (Fixed #43) (追記ここまで) Aug 19, 2025
Copy link
Owner

@ohkimur ohkimur left a comment

Choose a reason for hiding this comment

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

Thanks for the follow-up and for proposing a change. After our discussion in issue #43, I think this PR still adds complexity without fixing the core problem.

  • The exercise requires expanding actual tab characters ('\t') to the next tab stop, not interpreting the two-character sequence "\" + "t".
  • Replacing every tab with a fixed 8 spaces is still incorrect mid-line; the number of spaces must depend on the current column.

Suggestion to align with K&R 1.20:

  • Track the current column.
  • On '\t', output TABSIZE - (col % TABSIZE) spaces and increment col by that amount.
  • On '\n', reset col to 0.
  • Everything else: emit the character and increment col.

This keeps the solution minimal, correct, and easy to reason about. If you update the PR with this approach (TABSIZE as a macro is fine), I’m happy to re-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ohkimur ohkimur ohkimur requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /