Opened 8 years ago
Closed 8 years ago
#12288 closed defect (fixed)
hg_sage and friends should unset HGPLAIN
| Reported by: | jhpalmieri | Owned by: | jason |
|---|---|---|---|
| Priority: | critical | Milestone: | sage-4.8 |
| Component: | misc | Keywords: | |
| Cc: | Merged in: | sage-4.8.rc0 | |
| Authors: | John Palmieri | Reviewers: | Karl-Dieter Crisman |
| Report Upstream: | N/A | Work issues: | |
| Branch: | Commit: | ||
| Dependencies: | Stopgaps: |
Description (last modified by )
This is a followup to #12058. When you run hg_sage.log() in Sage, it runs the command hg which has been installed in SAGE_LOCAL/bin. It does not run sage --hg. Therefore, because of the changes in #12058, the variable HGPLAIN is set to "yes", so the pager is turned off. We should instead either run sage --hg or unset HGPLAIN when running commands using hg_sage.
Apply trac_12288-sage-hg.v2.patch.
Attachments (4)
Change History (14)
Changed 8 years ago by
comment:1 Changed 8 years ago by
- Milestone changed from sage-5.0 to sage-4.8
- Priority changed from major to critical
comment:2 Changed 8 years ago by
- Description modified (diff)
- Status changed from new to needs_review
I think that approach 1 is better, since then we can further modify how Sage's hg is used by customizing the sage-sage script, as in #12058.
comment:3 follow-up: ↓ 5 Changed 8 years ago by
- Reviewers set to Karl-Dieter Crisman
Is there any particular reason to do approach 2? I'm realizing that this would only turn up in log or diff with a long diff.
Anyway, approach 1 certainly fixes the issue for log and diff. Could there be any other consequences?
comment:4 Changed 8 years ago by
I agree that approach 1 is better.
comment:5 in reply to: ↑ 3 Changed 8 years ago by
Replying to kcrisman:
Is there any particular reason to do approach 2?
I don't think so. Both approaches occurred to me, they were both easy to implement, so I wrote both. Then I decided that I liked approach 1 better, but I also thought I should put them both here, in case someone thought of a good reason to use approach 2 instead.
I'm posting a new version of the first patch; the only difference is a comment explaining what's going on. (If people decide that approach 2 is better, we should add a similar comment to that, too.)
comment:6 Changed 8 years ago by
- Status changed from needs_review to needs_work
Obviously, some doctests in sage/misc/hg.py need to be fixed.
comment:7 Changed 8 years ago by
- Description modified (diff)
- Status changed from needs_work to positive_review
Okay, this was easy to fix. Apply trac_12288-sage-hg.patch and trac_12288-reviewer.patch. Passes all tests in sage/misc.
comment:8 Changed 8 years ago by
Here's a new patch which also fixes the output of doctests marked "not tested", not just the ones which failed before. Can we use this one instead?
comment:9 Changed 8 years ago by
- Description modified (diff)
Ok, seems fine to me, and applies/passes/looks nice/etc.
comment:10 Changed 8 years ago by
- Description modified (diff)
- Merged in set to sage-4.8.rc0
- Resolution set to fixed
- Status changed from positive_review to closed
version 2: disable HGPLAIN when running Mercurial from within Sage