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 Issue #11: Print new incoming message on a newline #50
Conversation
Hello @vitokhangnguyen! Thanks for updating this PR. There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2019-11-03 07:53:46 UTC |
99a341d
to
f3ad3c8
Compare
9a4b0d1
to
2c0b954
Compare
93f8726
to
f5558bc
Compare
@vitokhangnguyen I don't this is working correctly. Please test it locally again |
Hi @Haider8, can you be more specific on what went wrong? I am testing locally on both Windows and Linux subsystem. It seems to be ok for me. Edit: I am working on merging with the new master. I didn't know the master branch was updated. |
aa75d78
to
418ab3a
Compare
I have a few things fixes and merge with the current master. It works for me right now in both Windows and Linux subsystem. |
@vitokhangnguyen I think you didn't get a clear understanding of the issue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a different implementation to fix this issue!
Hi @Shanmugapriya03, Is this not what happens on your side? Please make sure you cloned this branch and did As for the implementation, could you clarify how different my current from your intention? |
I couldn't see the behaviour you explained above. Will try once again and let you know @vitokhangnguyen by tomorrow |
Clone and If the behavior is not as described above, please give me some feedback about how it is and how it is different from the original. |
@vitokhangnguyen This is the behavior I am getting |
I will take a look into that and give some changes/feedback soon. Thank you for your effort! |
I did not attempt to improve much but can anyone using a Windows system test this for me? It does work on my Windows system. |
@vitokhangnguyen Sorry, I don't have windows. Have you tried checking that when user1 is typing the message and then when another user sends the message it doesn't get printed right next to the message already typed by user1? |
Hi @Haider8 and @Shanmugapriya03, I am pushing a small change, hope that help. |
418ab3a
to
ebb3f32
Compare
ebb3f32
to
8dd5f83
Compare
Hi, @vitokhangnguyen I don't have a windows machine but this is the behavior I am getting and also authentication is not working. |
I am guessing so far this is a system-dependent thing. What I am using is the "\r" carriage return character to overwrite the current typing line with the incoming message. Maybe different systems render the "\r" character differently. I will find a way around this. May I know what is your current system or, specifically, what terminal are you using? |
do |
@Shanmugapriya03 Just for sure, do reinstall all package in requirements.txt:
with the new source code. |
@vitokhangnguyen This is the behavior I am getting @Haider8 is it working for you? |
@Shanmugapriya03 Thank you for testing out. |
@vitokhangnguyen Can you see the image above? I am in your branch. If it is old version of code then your branch has an old version |
I do see that. A very strange behavior because it is not the case on my side. Will continue looking into it. |
@Haider8 @Shanmugapriya03 Checkout to my branch if you are not already there:
Update your clone to be the same as mine:
Setup/rebuild the application (it is important that you do this after checking out to
Then you can test my work. Please give me some feedback after this if you can. |
@vitokhangnguyen Just to avoid confusions I deleted the folder from my system and cloned it again using this command Your fix kind of works but it gives 2 new major issues
@Haider8 Can you try it in your Ubuntu and let me know if you face similar issue... I tested it in my mac |
@Shanmugapriya03 Thanks for testing out, I have an idea on how to fix the cursor problem. It may be the different interpretation of a newline between Windows and Unix-like systems. |
b02bc62
to
337b19f
Compare
@Shanmugapriya03 I just made some small changes. Please pull from my repo/branch and build again before testing it out when you have time.
|
@Shanmugapriya03 Please test it. The issue of cursor shifting ahead is still present for me. |
@Haider8 This issue still exist for me too @vitokhangnguyen |
I will find a Mac system and do some testing/fixing on that. Thank you for your feedbacks. |
337b19f
to
b100677
Compare
Hi @Haider8 @Shanmugapriya03, |
@vitokhangnguyen Sorry for the delay in review. Your fix worked but introduced a new issue attaching a screenshot of the issue |
f42dee7
to
0e209cf
Compare
0e209cf
to
d4549ce
Compare
Hi @Shanmugapriya03, Thank you for pointing that out, I believe that has been fixed/ As I said, the current input module could use a lot of improvement. |
@vitokhangnguyen Works perfectly Fine! @Haider8 I am Approving this PR please do merge it |
This PR fixes issue #11. Two major problems it fixes: