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 for prompts containing newlines. #1105

Merged
merged 2 commits into from Dec 8, 2011

Conversation

takluyver
Copy link
Member

@rkern, I've tried this with the config example you supplied, and it seems to be working OK.

Specifically, we now only justify prompts based on the last line of the preceding prompt, and we don't attempt to justify multiline prompts at all.

Closes gh-1104

@rkern
Copy link
Contributor

rkern commented Dec 5, 2011

Works for me.

@takluyver
Copy link
Member Author

Great, thanks. I'll give the others a day or two to make any comments, then merge it.

@minrk
Copy link
Member

minrk commented Dec 6, 2011

works fine for me, thanks.

@fperez
Copy link
Member

fperez commented Dec 6, 2011

Glad to hear the fixes work correctly. I thought for a bit about what kind of automated test we could add to ensure this doesn't return to bite us later, but I can't quite seem to see a quick way to make one. I figured we could do one involving creating an irunner with custom prompts, running a session through it and validating back the output, but it sounds like a fair amount of hassle and I'm not sure the effort/payoff is worth it.

If you can think of a way to test this that isn't too painful to implement, go ahead and do it, otherwise merge as-is.

@takluyver
Copy link
Member Author

I think we could do a bit of unit testing with the .width attribute,
setting different prompts. But I'm not going to implement it before
sleeping. ;-)

@fperez
Copy link
Member

fperez commented Dec 6, 2011

Sure :) We've made good progress on the other PRs today, so we can sit on this one for a few days before the 0.12 rc.

@takluyver
Copy link
Member Author

I've added a few simple tests, and checked that they're passing.

@minrk
Copy link
Member

minrk commented Dec 8, 2011

nice, looks good to me.

@takluyver
Copy link
Member Author

Great. I'll merge it later today unless anyone objects.

@minrk
Copy link
Member

minrk commented Dec 8, 2011

please do, thanks!

@takluyver takluyver merged commit 96e8539 into ipython:master Dec 8, 2011
@takluyver
Copy link
Member Author

Rebased to avoid a merge, and pushed.

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.

Prompt spacing weird
4 participants