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

Download all attachments as: .zip

Change History (14)

Changed 8 years ago by jhpalmieri

version 2: disable HGPLAIN when running Mercurial from within Sage

comment:1 Changed 8 years ago by jdemeyer

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

comment:2 Changed 8 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 8 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 8 years ago by kini

I agree that approach 1 is better.

comment:5 in reply to: ↑ 3 Changed 8 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 8 years ago by jhpalmieri

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

comment:6 Changed 8 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 8 years ago by kcrisman

Apply after first solution patch

comment:7 Changed 8 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 8 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 8 years ago by jhpalmieri

apply instead of other patches

comment:9 Changed 8 years ago by kcrisman

  • Description modified (diff)

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

comment:10 Changed 8 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.