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

fix every cmd response is followed by % #697

Merged
merged 9 commits into from
Jun 12, 2024

Conversation

aditya028
Copy link
Contributor

@aditya028 aditya028 commented Jun 9, 2024

The % symbol at the end of every response is not actually part of the output . It's a feature of some terminal emulators, including the default terminal in macOS, which display a % or other symbol when the output from the last command does not end with a newline (\n).

Here is the output attached
Screenshot 2024-06-10 at 4 05 16 PM

Closes: #696

@vikash
Copy link
Contributor

vikash commented Jun 9, 2024

This just fixes the example and not the underlying problem.

@aditya028
Copy link
Contributor Author

okk i will look into it

@aditya028
Copy link
Contributor Author

please reivew it @vikash @srijan-27

@srijan-27
Copy link
Contributor

@aditya028 - Did you verified the changes by running the example? If yes, please attach the fixed terminal output.
Also, since now format is changed so some tests are failing, would need to fix them as well.

@aditya028
Copy link
Contributor Author

aditya028 commented Jun 10, 2024

@srijan-27 I have checked the examples and it works fine . Also, in the codebase it is mentioned there is some issue that needs to be resolved
TODO - provide proper exit codes here. Using os.Exit directly is a problem for tests.

Here is the output attached
Screenshot 2024-06-10 at 4 05 16 PM

@srijan-27
Copy link
Contributor

@aditya028 - Yes, that needs to be done someway. If you want you can dive deep and take a look on that, if not, then you can just fix the current tests.

@srijan-27
Copy link
Contributor

srijan-27 commented Jun 10, 2024

@aditya028 - Forgot to mention previously, if you work on the TODO task, then create a separate issue and PR for that. As it has separate context than this PR.

@aditya028
Copy link
Contributor Author

ok sure @srijan-27 , for now i will fix the test case for this .

@aditya028
Copy link
Contributor Author

@srijan-27 can you check this

@srijan-27
Copy link
Contributor

@srijan-27 can you check this

@aditya028 - Couple of tests are still failing.. you can check them from the workflows.

@srijan-27 srijan-27 merged commit 373d5a5 into gofr-dev:development Jun 12, 2024
11 checks passed
@aryanmehrotra aryanmehrotra mentioned this pull request Jun 18, 2024
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.

CMD response is followed by %
4 participants