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.

Sign up for GitHub

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

Jump to bottom

bush: fix links to line numbers and syntax highlighting #2122

Merged
merged 3 commits into from
Nov 13, 2017
Merged

bush: fix links to line numbers and syntax highlighting #2122

merged 3 commits into from
Nov 13, 2017

Conversation

oliver-sanders
Copy link
Member

  • When a line number is specified in the URL the line will be highlighted on page load.
  • Selecting a line no longer requires a content reload.
  • Highlighting of logger level information (i.e. INFO, DEBUG, ...) is now restored (broken by the new logging system).

@matthewrmshin please assign second review when the time comes.

@oliver-sanders oliver-sanders added this to the next-release milestone Oct 20, 2017
@oliver-sanders oliver-sanders self-assigned this Oct 20, 2017
@matthewrmshin
Copy link
Member

./t/rose-suite-restart/02-basic.t test failure most likely caused by cylc/cylc-flow#2437.

@matthewrmshin
Copy link
Member

./t/rose-bush/00-basic.t failure caused by server returning 500. Server err file gives:

  File "/home/matt/rose.git/lib/html/template/rose-bush/view.html", line 68, in template
    'replace', 'CRITICAL', '<span class="nocode text-dander">CRITICAL</span>') | map(
TemplateAssertionError: no filter named 'map'

@oliver-sanders
Copy link
Member Author

./t/rose-bush/00-basic.t now passing!

oliver-sanders
oliver-sanders commented Oct 23, 2017
View reviewed changes
lib/html/static/js/search.js Outdated
num = parseInt(num);

if (previous_search) {
loadFile("", num)
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that https://github.com/oliver-sanders/rose/pull/5/files#diff-cb9830bbb3b655703396b58b67a92e6b adds javascript syntax checking to the test battery for sphinx/_static this could be expanded to lib/html/static/js as well.

@metomi metomi deleted a comment Oct 23, 2017
@metomi metomi deleted a comment Oct 23, 2017
@metomi metomi deleted a comment Oct 30, 2017
@metomi metomi deleted a comment Nov 3, 2017
@matthewrmshin
Copy link
Member

@arjclark do you mind giving this one a quick sanity check?

arjclark
arjclark approved these changes Nov 13, 2017
View reviewed changes
Copy link
Contributor

@arjclark arjclark left a comment

Choose a reason for hiding this comment

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

Looks suitably sane to me. Assume we're ignoring the codacy bot comments as spurious?

@matthewrmshin
Copy link
Member

@arjclark You know what I'd normally say about CamelCase! (OK, I do accept that we should update our JavaScript code to the standard practice, rightly or wrongly, but that should not be the concern of this PR.)

@arjclark arjclark merged commit a0dafdc into metomi:master Nov 13, 2017
@oliver-sanders oliver-sanders deleted the bush-fix-line-number-links branch December 14, 2017 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arjclark arjclark arjclark approved these changes

@matthewrmshin matthewrmshin matthewrmshin approved these changes

Assignees

@oliver-sanders oliver-sanders

Labels
bug
Projects
None yet
Milestone
2018.02.0
Development

Successfully merging this pull request may close these issues.
3 participants
@oliver-sanders @matthewrmshin @arjclark