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 for text echo issue when running sbt. #3507

Merged
merged 3 commits into from Sep 14, 2017

Conversation

Projects
None yet
4 participants
@kczulko
Contributor

kczulko commented Sep 8, 2017

Fixes #3453

@lightbend-cla-validator

This comment has been minimized.

Show comment
Hide comment
@lightbend-cla-validator

lightbend-cla-validator Sep 8, 2017

Hi @kczulko,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

lightbend-cla-validator commented Sep 8, 2017

Hi @kczulko,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

@kczulko

This comment has been minimized.

Show comment
Hide comment
@kczulko

kczulko Sep 8, 2017

Contributor

Hi People :)

This is my very first sbt contribution. Since this lack of echo was pissing me off and I found the unsolved issue for that, I'd played a little bit around sbt source code and made this dummy fix. I'm almost sure it is not the right way to resolve that ( :) ) but I thought it would be great to start the discussion here.

Best regards,
Karol

Contributor

kczulko commented Sep 8, 2017

Hi People :)

This is my very first sbt contribution. Since this lack of echo was pissing me off and I found the unsolved issue for that, I'd played a little bit around sbt source code and made this dummy fix. I'm almost sure it is not the right way to resolve that ( :) ) but I thought it would be great to start the discussion here.

Best regards,
Karol

@lightbend-cla-validator

This comment has been minimized.

Show comment
Hide comment
@lightbend-cla-validator

lightbend-cla-validator Sep 8, 2017

Hi @kczulko,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

lightbend-cla-validator commented Sep 8, 2017

Hi @kczulko,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

@eed3si9n

This comment has been minimized.

Show comment
Hide comment
@eed3si9n

eed3si9n Sep 10, 2017

Member

Hi Karol, welcome to sbt project. I really appreciate the "let me fix it myself" spirit.

Member

eed3si9n commented Sep 10, 2017

Hi Karol, welcome to sbt project. I really appreciate the "let me fix it myself" spirit.

@eed3si9n eed3si9n added this to the 1.0.2 milestone Sep 10, 2017

@lightbend-cla-validator

This comment has been minimized.

Show comment
Hide comment
@lightbend-cla-validator

lightbend-cla-validator Sep 11, 2017

Hi @kczulko,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

lightbend-cla-validator commented Sep 11, 2017

Hi @kczulko,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

@kczulko

This comment has been minimized.

Show comment
Hide comment
@kczulko

kczulko Sep 11, 2017

Contributor

@eed3si9n so before returning a result I called t.restore and as you wrote, that also fixed the problem. Can you please guide me whether I can put some test for that? Would be great to eliminate that behavior once and for all. Thx in advance :)

Contributor

kczulko commented Sep 11, 2017

@eed3si9n so before returning a result I called t.restore and as you wrote, that also fixed the problem. Can you please guide me whether I can put some test for that? Would be great to eliminate that behavior once and for all. Thx in advance :)

@eed3si9n

This comment has been minimized.

Show comment
Hide comment
@eed3si9n

eed3si9n Sep 12, 2017

Member

Can you please guide me whether I can put some test for that? Would be great to eliminate that behavior once and for all. Thx in advance :)

I am not sure. Is there a way to programmatically observe that the terminal is echoing, in somewhat cross platform way?

Member

eed3si9n commented Sep 12, 2017

Can you please guide me whether I can put some test for that? Would be great to eliminate that behavior once and for all. Thx in advance :)

I am not sure. Is there a way to programmatically observe that the terminal is echoing, in somewhat cross platform way?

@eed3si9n eed3si9n changed the base branch from 1.x to 1.0.x Sep 12, 2017

@lightbend-cla-validator

This comment has been minimized.

Show comment
Hide comment
@lightbend-cla-validator

lightbend-cla-validator Sep 14, 2017

Hi @kczulko,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

lightbend-cla-validator commented Sep 14, 2017

Hi @kczulko,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

@kczulko

This comment has been minimized.

Show comment
Hide comment
@kczulko

kczulko Sep 14, 2017

Contributor

@eed3si9n,
Ok. I thought that the root cause of that issue was calling t.restore before returning a value. I've found that usingTerminal function has four usages. Three of them are taking height/widht of the terminal and one of them (LineReader::createReader) ignores terminal (t) parameter value. I've fixed that in the way you want so now t.restore is called both before and after f(t) call.

Contributor

kczulko commented Sep 14, 2017

@eed3si9n,
Ok. I thought that the root cause of that issue was calling t.restore before returning a value. I've found that usingTerminal function has four usages. Three of them are taking height/widht of the terminal and one of them (LineReader::createReader) ignores terminal (t) parameter value. I've fixed that in the way you want so now t.restore is called both before and after f(t) call.

@eed3si9n

This comment has been minimized.

Show comment
Hide comment
@eed3si9n

eed3si9n Sep 14, 2017

Member

LGTM. Thanks for the contribution.

Member

eed3si9n commented Sep 14, 2017

LGTM. Thanks for the contribution.

@eed3si9n eed3si9n merged commit d81d054 into sbt:1.0.x Sep 14, 2017

0 of 2 checks passed

codacy/pr Not so good... This pull request quality could be better.
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@egp

This comment has been minimized.

Show comment
Hide comment
@egp

egp Sep 19, 2017

Hooray. Thank you so much!

egp commented Sep 19, 2017

Hooray. Thank you so much!

@kczulko kczulko deleted the kczulko:issue-3453 branch Sep 20, 2017

mrb24 added a commit to UBOdin/mimir that referenced this pull request Sep 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment