Skip to content

Simplify convoluted nested condition logic in text handler#59

Merged
adcondev merged 3 commits intofeat/jsonfrom
copilot/sub-pr-58
Nov 7, 2025
Merged

Simplify convoluted nested condition logic in text handler#59
adcondev merged 3 commits intofeat/jsonfrom
copilot/sub-pr-58

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Nov 7, 2025

Descripción

Refactors nested conditional logic in handleText that was flagged as convoluted and difficult to understand. The original implementation had a confusing double-nested condition where the outer checked cmd.NewLine || cmd.Content == "" and the inner checked cmd.NewLine || (cmd.Content != "" && !cmd.NewLine).

Before:

if cmd.NewLine || cmd.Content == "" {
    if cmd.NewLine || (cmd.Content != "" && !cmd.NewLine) {
        if err := printer.PrintLine(cmd.Content); err != nil {
            return err
        }
    }
} else {
    if err := printer.Print(cmd.Content); err != nil {
        return err
    }
}

After:

if cmd.Content != "" {
    if cmd.NewLine {
        if err := printer.PrintLine(cmd.Content); err != nil {
            return err
        }
    } else {
        if err := printer.Print(cmd.Content); err != nil {
            return err
        }
    }
}

The refactored version makes the logic explicit: skip empty content, use PrintLine for newline-terminated content, otherwise use Print. Style reset logic remains unchanged and executes in all cases.

Tipo de cambio

  • 🐛 Bug fix
  • ✨ Nueva funcionalidad
  • 📚 Solo documentación
  • 🔧 Configuración
  • ♻️ Refactoring
  • 📦 Dependencias

¿Cómo se ha probado?

  • Tests automáticos pasan
  • Probado manualmente
  • N/A (solo docs/config)

Checklist

  • El código sigue las convenciones del proyecto
  • Los tests pasan localmente
  • He actualizado la documentación si era necesario

Notas adicionales

Addresses feedback from PR #58: #58 (comment)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI self-assigned this Nov 7, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 7, 2025

👋 Thanks for opening this PR, @Copilot!

Here's what will happen next:

  • 🤖 Automated checks will run
  • 🏷️ Labels will be added automatically
  • 👀 A maintainer will review your changes

Please make sure:

  • ✅ All tests pass
  • 📝 The PR title follows conventional commits
  • 📋 The PR template is filled out completely

Copilot AI and others added 2 commits November 7, 2025 18:25
Co-authored-by: adcondev <38170282+adcondev@users.noreply.github.com>
Co-authored-by: adcondev <38170282+adcondev@users.noreply.github.com>
Copilot AI changed the title [WIP] Address feedback on document building features Simplify convoluted nested condition logic in text handler Nov 7, 2025
Copilot AI requested a review from adcondev November 7, 2025 18:29
@adcondev adcondev marked this pull request as ready for review November 7, 2025 18:36
Copilot AI review requested due to automatic review settings November 7, 2025 18:36
Copy link
Copy Markdown
Owner

@adcondev adcondev left a comment

Choose a reason for hiding this comment

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

ok

@adcondev adcondev merged commit 34be4b8 into feat/json Nov 7, 2025
29 checks passed
@adcondev adcondev deleted the copilot/sub-pr-58 branch November 7, 2025 18:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR simplifies the text printing logic in the handleText function by removing confusing nested conditional statements and replacing them with a clearer, more maintainable structure.

  • Refactored the text printing conditional logic from nested if-statements to a straightforward structure
  • The new implementation checks if content exists first, then decides whether to print with or without a newline based on the NewLine flag

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/document/handlers.go
if err := printer.Print(cmd.Content); err != nil {
return err
}
}
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

This refactoring changes the behavior when cmd.Content is empty but cmd.NewLine is true. The previous code would print a blank line in this case, but the new code skips printing entirely. While the new logic is cleaner and more intuitive (using FeedCommand for blank lines), this behavioral change could affect existing code or JSON documents that rely on printing blank lines via text commands with empty content. Consider whether this breaking change is intentional and if it requires documentation or a migration guide.

Suggested change
}
}
} else if cmd.NewLine {
// Mantener compatibilidad: imprimir línea en blanco si Content está vacío y NewLine es true
if err := printer.PrintLine(""); err != nil {
return err
}

Copilot uses AI. Check for mistakes.
adcondev added a commit that referenced this pull request Nov 7, 2025
* fix(encoding): improve logging for unsupported code tables

Enhance the logging messages for unsupported encoding and code tables
to provide clearer context. This change helps in debugging and
understanding fallback behavior to Windows-1252.

- Updated log messages to specify unsupported code tables
- Ensured proper handling of character code table selection

Signed-off-by: Adrián Constante <ad_con.reload@proton.me>

* feat(document): add document building and execution features

- Introduce a new Builder for creating print documents
- Implement an Executor to handle document execution
- Support commands for text, images, separators, feeds, and cuts
- Add JSON parsing and serialization for documents

This enhances the library's capability to build and print
documents programmatically or from JSON files.

Signed-off-by: Adrián Constante <ad_con.reload@proton.me>

* refactor(graphics): improve variable naming for clarity

Updated variable names in base64_example.go and escpos_profile.go
to enhance code readability. Removed unused font mappings in
escpos_profile.go to streamline the profile structure.

Signed-off-by: Adrián Constante <ad_con.reload@proton.me>

* docs(pkg): add motivation documents for project overview

- Introduce MOTIVATION1.md and MOTIVATION2.md files
- Provide strategic analysis and architectural proposals
- Enhance understanding of the ESC/POS library development

Signed-off-by: Adrián Constante <ad_con.reload@proton.me>

* fix(escpos): simplify convoluted nested condition logic in text handler (#59)

* Initial plan

* refactor(document): simplify nested condition logic in text handler

Co-authored-by: adcondev <38170282+adcondev@users.noreply.github.com>

* fix(document): ensure styles are always reset after text command

Co-authored-by: adcondev <38170282+adcondev@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: adcondev <38170282+adcondev@users.noreply.github.com>

* fix(converter): add error logging for JSON marshaling

This enhancement will aid in debugging and ensure that issues are captured and reported effectively.

Signed-off-by: Adrián Constante <ad_con.reload@proton.me>

---------

Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants