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

Use templates for CSS #6865

Closed
TimDumol mannequin opened this issue Sep 2, 2009 · 20 comments
Closed

Use templates for CSS #6865

TimDumol mannequin opened this issue Sep 2, 2009 · 20 comments

Comments

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Sep 2, 2009

CSS is currently served by css() in css.py, even though it is completely static. Using templates for it should make things easier to customize in the future.

Component: notebook

Keywords: notebook css stylesheets

Author: Tim Dumol

Reviewer: Mitesh Patel, Jason Grout

Merged: Sage 4.1.2.alpha4

Issue created by migration from https://trac.sagemath.org/ticket/6865

@TimDumol TimDumol mannequin assigned boothby Sep 2, 2009
@TimDumol
Copy link
Mannequin Author

TimDumol mannequin commented Sep 2, 2009

Moved CSS in css.py to templates, and adjusted twist.py to use them.

@TimDumol
Copy link
Mannequin Author

TimDumol mannequin commented Sep 2, 2009

Author: Tim Dumol

@TimDumol
Copy link
Mannequin Author

TimDumol mannequin commented Sep 2, 2009

comment:1

Attachment: trac_6865-templates-css.patch.gz

@TimDumol TimDumol mannequin added this to the sage-4.1.2 milestone Sep 2, 2009
@TimDumol TimDumol mannequin added the s: needs review label Sep 2, 2009
@jasongrout
Copy link
Member

comment:2

This probably needs to be rebased after #6939 to include the CSS fix there (or the rebasing should go the other way, if this gets reviewed before that gets merged...)

@jasongrout
Copy link
Member

comment:3

#6940 is a dup of this ticket (i.e., close #6940).

@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 17, 2009

comment:4

Does the patch depend on another ticket's patch? Applying attachment: trac_6865-templates-css.patch to 4.1.2.alpha1, I get

applying trac_6865-templates-css.patch
patching file sage/server/notebook/css.py
Hunk #1 FAILED at 0
1 out of 3 hunks FAILED -- saving rejects to file sage/server/notebook/css.py.rej
patching file sage/server/notebook/twist.py
Hunk #1 FAILED at 133
Hunk #2 succeeded at 1787 with fuzz 1 (offset 37 lines).
Hunk #3 FAILED at 2315
2 out of 3 hunks FAILED -- saving rejects to file sage/server/notebook/twist.py.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh trac_6865-templates-css.patch

The important one is the last, which doesn't apply because the local twist.py contains

        elif self.problem == 'suspended':
            return HTMLResponse(stream = message("Your account is currently suspended."))

But I could have altered my configuration.

@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 18, 2009

comment:5

I think we need to add

include sage/server/notebook/css/*

to MANIFEST.in, or we'll definitely hear from the release manager...

@jasongrout
Copy link
Member

comment:6

What is MANIFEST.in and when do we need to add things to it?

@jasongrout
Copy link
Member

comment:7

(setting to "needs work", based on mpatel's comment about MANIFEST.in and his comment about applying the patch)

@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 19, 2009

comment:8

Replying to @jasongrout:

What is MANIFEST.in and when do we need to add things to it?

I believe the sage -bdist and sage -sdist invoke distutils to build sage-*.spkg. Distutils uses SAGE_ROOT/devel/sage/MANIFEST.in to determine which files to include in the distribution. If we add files to the local Mercurial repository, we should check that some line(s) in MANIFEST.in includes the newly tracked files or add a new line. Otherwise, the next (pre-)release of Sage will not include them. As mvngu puts it, this results in "repository corruption".

Previous sightings: #4460, #5653, #5799, #6568, etc.

@TimDumol
Copy link
Mannequin Author

TimDumol mannequin commented Sep 20, 2009

comment:9

No, this doesn't depend on other patches. It is based on 4.1.2.alpha1.

I've made the requisite changes to MANIFEST.in.

@TimDumol TimDumol mannequin added s: needs review and removed s: needs work labels Sep 20, 2009
@TimDumol
Copy link
Mannequin Author

TimDumol mannequin commented Sep 20, 2009

Moved CSS in css.py to templates, and adjusted twist.py to use them. Rev 2. Apply this patch only.

@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 21, 2009

Attachment: trac_6865-templates-css.2.patch.gz

Attachment: trac_6865-templates-css.3.patch.gz

Moved CSS in css.py to templates, and adjusted twist.py to use them. Rev 3. Apply this patch only.

@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 21, 2009

comment:10

Changes in patch v3:

  • css.py: if os.path.exists(user_css): --> if os.path.exists(user_css_path):
  • css.py: + --> os.path.join()
  • css.py: html -> HTML in docstring
  • main.css: {% -> {%-

Sorry about the trivial stuff. I just noticed the whitespace control part of the Jinja2 docs.

This is a positive review from me, but someone should review my changes.

@qed777 qed777 mannequin changed the title Use templates for CSS [partial positive review] Use templates for CSS Sep 21, 2009
@jasongrout
Copy link
Member

comment:11

Your changes look good to me.

@jasongrout jasongrout changed the title [partial positive review] Use templates for CSS Use templates for CSS Sep 22, 2009
@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Sep 22, 2009

Merged: Sage 4.1.2.alpha3

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Sep 22, 2009

Reviewer: Mitesh Patel, Jason Grout

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Sep 22, 2009

comment:13

Merged trac_6865-templates-css.3.patch.

@sagetrac-mvngu sagetrac-mvngu mannequin closed this as completed Sep 22, 2009
@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Sep 27, 2009

comment:14

There is no 4.1.2.alpha3. Sage 4.1.2.alpha3 was William Stein's release for working on the making the notebook a standalone package.

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Sep 27, 2009

Changed merged from Sage 4.1.2.alpha3 to Sage 4.1.2.alpha4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants