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

feat: Added support for ANSI Colours in Drifty CLI #329

Merged
merged 17 commits into from
Nov 8, 2023

Conversation

J9414
Copy link
Contributor

@J9414 J9414 commented Nov 6, 2023

Fixes issue

Fixes #284

Changes proposed

As you suggested, colors for each message type are:

INFO: Blue or Green
WARN: Yellow
ERROR: Red

Screenshots

Capture1

Capture1
Capture2

Note to reviewers

No response

Copy link

vercel bot commented Nov 6, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
drifty ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 8, 2023 2:33pm

@github-actions github-actions bot added the App 💻 Issues/Pull Requests which update Drifty Application Code label Nov 6, 2023
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Yeah! You did it 🎉 Now, Relax 😉, Grab a drink ☕, and wait for the maintainers to check your contributions. Meanwhile, you can discuss on other issues and solve them 😀. Thank You 😃!

Meanwhile you can also discuss about the project in our Discord Server 😀

trying to get rid of ES lint error by checkstyle
added white space in between added text for colour ex GREEN + insted of GREEN+
@SaptarshiSarkar12
Copy link
Owner

@J9414 Please comment in the issue so that I can assign you with the task.

more chages as needed
@SaptarshiSarkar12 SaptarshiSarkar12 changed the title Update Main.java added color in CLI text Added support for ANSI Colours in Drifty CLI Nov 7, 2023
@SaptarshiSarkar12 SaptarshiSarkar12 added the feature ✨ New feature request or addition label Nov 7, 2023
@SaptarshiSarkar12 SaptarshiSarkar12 linked an issue Nov 7, 2023 that may be closed by this pull request
Copy link
Owner

@SaptarshiSarkar12 SaptarshiSarkar12 left a comment

Choose a reason for hiding this comment

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

@J9414 I think it would be better to add ANSI colouring in MessageBroker class. It can be included in the sendMessage() method of that class.
Using the Message type (INFO, WARN, ERROR) in that method, colours can be added accordingly.

taking all chage back
Copy link
Owner

@SaptarshiSarkar12 SaptarshiSarkar12 left a comment

Choose a reason for hiding this comment

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

@J9414 Please make the requested changes 👇

  • Please change tabs from 8 spaces to 4 spaces, in all code lines of MessageBroker class.
  • Please use switch statement instead of if, for the colour determining code block. Also, add a default case for that switch block - which will be only the message which no colours.

as per new requrment added swich case and changed space in IDE for space
@J9414
Copy link
Contributor Author

J9414 commented Nov 7, 2023

@J9414 Please make the requested changes 👇

  • Please change tabs from 8 spaces to 4 spaces, in all code lines of MessageBroker class.
  • Please use switch statement instead of if, for the colour determining code block. Also, add a default case for that switch block - which will be only the message which no colours.

-- I did chages in IDE for space but i think still that is giving error
-- I added swich, however in default case please let me know if i am right or not

Copy link
Owner

@SaptarshiSarkar12 SaptarshiSarkar12 left a comment

Choose a reason for hiding this comment

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

@J9414 Please make the requested changes 👇

  • Please keep the spaces in the GUI colour code block as it was before.
  • In the default case, it will be message = message.
  • Please use enhanced switch case statements.

Copy link
Owner

@SaptarshiSarkar12 SaptarshiSarkar12 left a comment

Choose a reason for hiding this comment

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

@J9414 Please use the switch statement like this 👇

message = switch (messageType) {
      case INFO -> "\033[92m" + message + "\033[0m";
      ........
}

After applying this, please check once if everything works perfectly. There might be a missing semi-colon in the above sample code.

added messge = switch {.. };
Copy link
Owner

@SaptarshiSarkar12 SaptarshiSarkar12 left a comment

Choose a reason for hiding this comment

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

@J9414 Please commit the below requested changes.

src/main/java/Utils/MessageBroker.java Outdated Show resolved Hide resolved
src/main/java/Utils/MessageBroker.java Outdated Show resolved Hide resolved
src/main/java/Utils/MessageBroker.java Outdated Show resolved Hide resolved
src/main/java/Utils/MessageBroker.java Outdated Show resolved Hide resolved
J9414 and others added 3 commits November 8, 2023 20:01
as needed

Co-authored-by: Saptarshi Sarkar <saptarshi.programmer@gmail.com>
Co-authored-by: Saptarshi Sarkar <saptarshi.programmer@gmail.com>
Co-authored-by: Saptarshi Sarkar <saptarshi.programmer@gmail.com>
Co-authored-by: Saptarshi Sarkar <saptarshi.programmer@gmail.com>
@J9414
Copy link
Contributor Author

J9414 commented Nov 8, 2023

@J9414 Please commit the below requested changes.

thanks for suggesting, i will try to improve on it

@SaptarshiSarkar12
Copy link
Owner

Welcome 😁.
@J9414 Please check if the latest code works on your machine and most of the lines of Drifty's outputs are coloured, and replace the latest screenshot in the first comment of this PR.

@SaptarshiSarkar12 SaptarshiSarkar12 changed the title Added support for ANSI Colours in Drifty CLI feat: Added support for ANSI Colours in Drifty CLI Nov 8, 2023
Copy link
Owner

@SaptarshiSarkar12 SaptarshiSarkar12 left a comment

Choose a reason for hiding this comment

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

@J9414 The colour codes doesn't seem to work as they are different as compared to the one given here. Please check once and make the requested changes.
Also, please post two screenshots one for normal coloured lines and the other for coloured bold lines.

@J9414
Copy link
Contributor Author

J9414 commented Nov 8, 2023

Welcome 😁. @J9414 Please check if the latest code works on your machine and most of the lines of Drifty's outputs are coloured, and replace the latest screenshot in the first comment of this PR.

This was my first pr and i managed to actually add some input thanks again and i added new screen shots

@J9414
Copy link
Contributor Author

J9414 commented Nov 8, 2023

@J9414 The colour codes doesn't seem to work as they are different as compared to the one given here. Please check once and make the requested changes. Also, please post two screenshots one for normal coloured lines and the other for coloured bold lines.

the colour i added are in // High Intensity section so i thout it would look much batter on CLI black background

@SaptarshiSarkar12
Copy link
Owner

@J9414 It's okay. No problem. I haven't checked the High Intensity section. Sorry for that.
Congratulations for your first PR 👏😀! Welcome to Open-Source!
Yes, you're right. The High Intensity colour actually looks good in the screenshots.

Copy link
Owner

@SaptarshiSarkar12 SaptarshiSarkar12 left a comment

Choose a reason for hiding this comment

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

Looks good to merge 👍.
Thanks for contributing 🚀 🚀.
You may join our Discord server - https://discord.gg/DeT4jXPfkG to get updates about the project.

@SaptarshiSarkar12 SaptarshiSarkar12 merged commit 180ddca into SaptarshiSarkar12:master Nov 8, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App 💻 Issues/Pull Requests which update Drifty Application Code feature ✨ New feature request or addition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add ANSI Colours to the Drifty CLI outputs
2 participants