Skip to content

Conversation

@duphyf
Copy link

@duphyf duphyf commented Nov 30, 2025

Let me remind you of the original text of the exercise:

Write a program to remove trailing blanks and tabs from each line of input, and to delete entirely blank lines.

Resolved issues:

  1. The remove_trailing_blanks function was not working correctly.

    Its task was to remove trailing blanks and tabs from the passed string, but the string does not always end with a newline character ('\n'). If it is the end of the file, it may end with EOF, i.e. "qweEOF", in which case there is no need to add the '\n' character.

    Also, in the remove_trailing_blanks logic, we initially started iterating through characters with i = length - 2, which again would behave incorrectly in the case of EOF.

    Try this input (EOF means, for example, Ctrl+D in the terminal):

    • "qweEOF" (the last character will be removed)
    • "qwe \t EOF" ('\n' will be added, which was not in the original string).
  2. The logic for "delete entirely blank lines" was not implemented.

    Try this input:

    • "\n"
    • "\t\t\t\n"

Summary by CodeRabbit

  • Refactor
    • Updated trailing blank removal function to return the new string length and properly handle newline preservation in processed lines.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 30, 2025

Walkthrough

Modified the remove_trailing_blanks function to return an int instead of void, now providing the new string length after stripping trailing whitespace. Added logic to track and preserve newlines, updated the call site in main to use the return value, and introduced macros for newline handling constants.

Changes

Cohort / File(s) Summary
Function signature and return type update
chapter_1/exercise_1_18/trailing_blanks.c
Changed remove_trailing_blanks return type from void to int, now returns the new length after removing trailing blanks. Added newline-tracking logic and HAS_NEWLINE/NO_NEWLINE macros. Updated main to assign and use the returned length.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Verify the backwards iteration logic correctly identifies and strips spaces, tabs, and newlines
  • Confirm the conditional newline preservation logic is sound and returns the correct new length
  • Check that the macro definitions (HAS_NEWLINE, NO_NEWLINE) are properly utilized
  • Validate that the main function correctly uses the returned length value

Poem

🐰 A function that once returned naught,
Now yields the length it has wrought,
With newlines preserved just right,
Trailing blanks stripped from sight,
Return values—what a thought!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Fix exercise 1-18' is vague and generic, using non-descriptive language that doesn't convey the specific changes made to the codebase. Use a more descriptive title that explains the main change, such as 'Fix remove_trailing_blanks to handle EOF without newline and delete blank lines' or 'Update remove_trailing_blanks to return length and handle lines without trailing newlines'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
chapter_1/exercise_1_18/trailing_blanks.c (1)

15-20: Logic is correct; consider minor readability improvement.

The implementation correctly captures the new length and skips printing blank lines (lines reduced to just "\n"). The condition len > 1 || line[0] != '\n' works correctly but could be made more explicit.

Consider this equivalent but more explicit condition:

-    if (len > 1 || line[0] != '\n')
+    if (!(len == 1 && line[0] == '\n'))
     {
       printf("%s", line);
     }

This makes it clearer that you're specifically filtering out lines that are exactly a single newline character.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2f1fed8 and ff98d26.

📒 Files selected for processing (1)
  • chapter_1/exercise_1_18/trailing_blanks.c (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
chapter_1/exercise_1_18/trailing_blanks.c (1)
chapter_1/exercise_1_19/reverse.c (1)
  • length (49-57)
🔇 Additional comments (3)
chapter_1/exercise_1_18/trailing_blanks.c (3)

6-6: LGTM! Function signature correctly updated.

The return type change from void to int allows the function to communicate the new string length after processing, which is essential for the correct implementation of blank line deletion.


46-47: LGTM! Macros improve code readability.

The HAS_NEWLINE and NO_NEWLINE macros make the intent clear and are appropriate for K&R C style. This is preferable to using magic numbers directly in the code.


49-72: Excellent fix! All PR objectives successfully addressed.

The implementation correctly handles both issues mentioned in the PR:

  1. EOF without newline: Starting at i = length - 1 instead of length - 2 correctly processes strings that don't end with '\n', fixing cases like "qwe" and "qwe \t " at EOF.

  2. Blank line deletion: Lines that contain only whitespace (spaces, tabs, newlines) are now reduced to "\n" (or empty string if no newline was present) and filtered appropriately by the main function.

The logic is sound:

  • Loop strips all trailing whitespace (including newline)
  • After the loop, i points to the last non-whitespace character (or -1 if all whitespace)
  • Newline is preserved when originally present
  • String is properly null-terminated in both code paths

Edge cases are handled correctly:

  • All whitespace with newline: " \t\n""\n" (suppressed)
  • All whitespace without newline: " \t""" (printed, but acceptable since no newline means it's not a complete line)
  • Mixed content: "abc \t\n""abc\n"
  • EOF cases: "abc""abc"

@ohkimur ohkimur closed this Dec 9, 2025
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