Opened 10 years ago

Closed 10 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:

Status badges

Description (last modified by jdemeyer)

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)

trac_12288-unset-HGPLAIN.patch (1.0 KB) - added by jhpalmieri 10 years ago.
version 2: disable HGPLAIN when running Mercurial from within Sage
trac_12288-sage-hg.patch (1.0 KB) - added by jhpalmieri 10 years ago.
version 1: use "sage --hg" instead of "hg" to run Mercurial
trac_12288-reviewer.patch (2.6 KB) - added by kcrisman 10 years ago.
Apply after first solution patch
trac_12288-sage-hg.v2.patch (10.0 KB) - added by jhpalmieri 10 years ago.
apply instead of other patches

Download all attachments as: .zip

Change History (14)

Changed 10 years ago by jhpalmieri

version 2: disable HGPLAIN when running Mercurial from within Sage

comment:1 Changed 10 years ago by jdemeyer

  • Milestone changed from sage-5.0 to sage-4.8
  • Priority changed from major to critical

comment:2 Changed 10 years ago by jhpalmieri

  • 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: Changed 10 years ago by kcrisman

  • 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 10 years ago by kini

I agree that approach 1 is better.

comment:5 in reply to: ↑ 3 Changed 10 years ago by jhpalmieri

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.)

Changed 10 years ago by jhpalmieri

version 1: use "sage --hg" instead of "hg" to run Mercurial

comment:6 Changed 10 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Obviously, some doctests in sage/misc/hg.py need to be fixed.

Changed 10 years ago by kcrisman

Apply after first solution patch

comment:7 Changed 10 years ago by kcrisman

  • 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 10 years ago by jhpalmieri

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?

Changed 10 years ago by jhpalmieri

apply instead of other patches

comment:9 Changed 10 years ago by kcrisman

  • Description modified (diff)

Ok, seems fine to me, and applies/passes/looks nice/etc.

comment:10 Changed 10 years ago by jdemeyer

  • Description modified (diff)
  • Merged in set to sage-4.8.rc0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.