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

Replace termcolor with colored module #40

Merged
merged 1 commit into from
Oct 7, 2022

Conversation

GauthamBellamkonda
Copy link
Contributor

Fixes #37

Add new option to the CLI for checkered board
Use colored module for printing the board
Update README

Copy link
Collaborator

@ClasherKasten ClasherKasten left a comment

Choose a reason for hiding this comment

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

Unfortunately there are several problems with this implementation:

  • when using chess -c, the pieces don't get printed in the center of the square
    cl-chess_bug#40
  • when using chess -cu some parts of the board don't get printed at all
    cl-chess_bug#40 1

@GauthamBellamkonda
Copy link
Contributor Author

GauthamBellamkonda commented Oct 4, 2022

Hey @ClasherKasten,

I'm not able to reproduce the second bug. Using chess -cu prints a complete board on my machine. I'm using the standard terminal on Ubuntu 20.04.

image

For the first bug, we could use full width unicode characters (they basically occupy twice the width of a normal character) to represent pieces on the board. I had to

  • replace the triangle character '▲' for a pawn with a full-width 'p', and
  • replace the full stop characters with a space for empty cells.

It finally looks like this

image

Let me know if this is good enough, or if you have any better ideas/proposals.

@ClasherKasten
Copy link
Collaborator

ClasherKasten commented Oct 7, 2022

I'm not able to reproduce the second bug. Using chess -cu prints a complete board on my machine. I'm using the standard terminal on Ubuntu 20.04.

Interestingly I couldn't reproduce it myself. Don't know what exactly this was, but let's say it's non-existing for now.

For the first bug, we could use full width unicode characters (they basically occupy twice the width of a normal character) to represent pieces on the board.

The whole idea of a separated unicode mode was to also let people play this when their terminal has no proper unicode support.
So it wouldn't make any sense replacing the normal characters through unicode characters.
I think it is almost impossible to achieve a good looking result without unicode characters.

The plan I have now in mind would be to remove the -u switch and fully convert the application to unicode only.
I'll open a new issue for removing non-unicode mode.

But before I merge this, it would be nice to switch out the dependencies in the requirements.txt and setup.py files

And some small note on the side: I probably gonna merge #41 first because it is a way bigger change and I think if I would merge this PR first, it would eventually result in merge conflicts.

@ClasherKasten
Copy link
Collaborator

ClasherKasten commented Oct 7, 2022

I felt so free to resolve the merge conflicts created by merging #43 and #41

Add new option to the CLI for checkered board
Use colored module for printing the board
Update README
@ClasherKasten ClasherKasten merged commit e0903dc into marcusbuffett:master Oct 7, 2022
@ClasherKasten ClasherKasten added the hacktoberfest-accepted Label for accepted PRs while Hacktoberfest label Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Label for accepted PRs while Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace termcolor
2 participants