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

Set continuous scroll in tests #62

Merged
merged 1 commit into from
Feb 9, 2022

Conversation

jkingdon
Copy link
Collaborator

@jkingdon jkingdon commented Feb 9, 2022

That is, have the test harness prepend "set scroll continuous" to
the .in file.

Update all .expected files to include "Continuous scrolling is now in effect."

This follows up on discussion in #58 and #59

That is, have the test harness prepend "set scroll continuous" to
the .in file.

Update all .expected files to include
"Continuous scrolling is now in effect."
Copy link
Member

@digama0 digama0 left a comment

Choose a reason for hiding this comment

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

Still not sure this is the direction we should take, but the contents of the PR itself look fine.

@jkingdon
Copy link
Collaborator Author

jkingdon commented Feb 9, 2022

Still not sure this is the direction we should take, but the contents of the PR itself look fine.

🤷 Yeah, it isn't really a slam dunk for me either. I sort of go back and forth about whether my being wary of isatty is just my own aesthetic distate, or whether avoiding isatty for a little while until we make our mind is the thing to do, or whether it'll be fine and we'll come up with a Windows fix without too much trouble or whether we should do the Windows fix first (dropping Windows support, which I guess #59 does as currently written, seems like a big step) or what.

I guess we can give it a day or a few to see if more people weigh in - it has been less than 24 hours since we opened this discussion.

@jkingdon jkingdon merged commit 296f66c into metamath:master Feb 9, 2022
@jkingdon jkingdon deleted the continuous-test branch February 9, 2022 14:28
@jkingdon
Copy link
Collaborator Author

jkingdon commented Feb 9, 2022

OK, I'm glad to see some activity on #59 and #64 but the results there are..... well maybe I'll just say doesn't seem to me like we should rush into the #59 fix. Therefore, I will merge this at least for now. We can always revert later if we want.

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.

2 participants