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

Fixing adoc out-of-sequence warnings for pico-sdk #2963

Merged
merged 5 commits into from
Oct 17, 2023
Merged

Fixing adoc out-of-sequence warnings for pico-sdk #2963

merged 5 commits into from
Oct 17, 2023

Conversation

nelliemckesson
Copy link
Contributor

Fixes #2828 . @lurch give it a try and let me know if you see anything unexpected!

@aallan aallan added bug fix toolchain This is an infrastructure/toolchain issue labels May 23, 2023
@lurch
Copy link
Contributor

lurch commented May 30, 2023

Hi Nellie, I've just given this a spin, and I don't think it's what we want.

In the current (live) version the LHS menu for https://www.raspberrypi.com/documentation/pico-sdk/high_level.html looks like this, which is how @kilograham wants it to be:
Screenshot from 2023-05-30 16-12-08

But when testing this PR it looks like this, which Graham said was "wrong" when this was how https://datasheets.raspberrypi.com/pico/raspberry-pi-pico-c-sdk.pdf displayed things. (note that mutex no longer appears "beneath" pico_sync in the hierarchy)
Screenshot from 2023-05-30 16-14-19

@lurch lurch marked this pull request as draft June 13, 2023 07:53
@lurch
Copy link
Contributor

lurch commented Jun 13, 2023

(I've just converted this to draft, to prevent it being merged prematurely)

@nelliemckesson
Copy link
Contributor Author

@lurch just pushed a change that fixes the issue you pointed out -- how do things look now?

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@aallan
Copy link
Contributor

aallan commented Aug 20, 2023

Still needs doing.

@aallan
Copy link
Contributor

aallan commented Sep 4, 2023

Is this ready to merge?

@lurch
Copy link
Contributor

lurch commented Sep 5, 2023

@nelliemckesson I've not looked at this, and I'm afraid I don't have time to at the moment 😕

@aallan
Copy link
Contributor

aallan commented Sep 5, 2023

@kilograham Don't suppose you might have time to take a look and check whether this does what you want it to do?

@aallan aallan marked this pull request as ready for review September 5, 2023 15:01
@aallan aallan added the waiting for comment Awaiting a response label Sep 16, 2023
@lurch
Copy link
Contributor

lurch commented Sep 25, 2023

@nelliemckesson I've just pulled and built this branch, and I still get a load of out-of-sequence warnings? (which is what this PR was supposed to fix?) 🤔

$ make
scripts/create_auto_ninjabuild.py documentation/index.json _config.yml documentation/asciidoc scripts build/jekyll build/adoc_includes jekyll-assets build-pico-sdk-docs documentation/redirects documentation/images build/autogenerated.ninja
ninja
[1772/1772] scripts/create_htaccess.py...ation/redirects build/jekyll/.htaccess
bundle exec jekyll build
Configuration file: /home/andrew/github/raspberrypi/documentation2/_config.yml
            Source: /home/andrew/github/raspberrypi/documentation2/build/jekyll
       Destination: /home/andrew/github/raspberrypi/documentation2/documentation/html
 Incremental build: disabled. Enable with --incremental
      Generating... 
       Jekyll Feed: Generating feed for posts
asciidoctor: WARNING: group__alarm.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__async__context__freertos.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__async__context__poll.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__async__context__threadsafe__background.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__channel__config.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__critical__section.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__cyw43__driver.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__cyw43__ll.adoc: line 9: section title out of sequence: expected level 1, got level 3
asciidoctor: WARNING: group__interp__config.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__lock__core.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__multicore__fifo.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__multicore__lockout.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__mutex.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__pico__btstack__cyw43.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__pico__lwip__arch.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__pico__lwip__freertos.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__pico__lwip__nosys.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__pico__stdio__semihosting.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__pico__stdio__uart.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__pico__stdio__usb.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__pio__instructions.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__queue.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__repeating__timer.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__sem.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__sleep.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__sm__config.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__timestamp.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__util__datetime.adoc: line 9: section title out of sequence: expected level 1, got level 2
asciidoctor: WARNING: group__util__pheap.adoc: line 9: section title out of sequence: expected level 1, got level 2
                    done in 13.3 seconds.
 Auto-regeneration: disabled. Use --watch to enable.

@aallan aallan added waiting for revisions Waiting for the OP to make revisions and removed waiting for comment Awaiting a response labels Sep 29, 2023
@nelliemckesson
Copy link
Contributor Author

@lurch So, in draft 1 I was shooting for a "good enough" result -- reducing the number of warnings by > 50%. I went ahead and added functionality to completely remove them.

For that final 50% reduction, the reason they were occurring was because included adoc files were being BOTH correctly included into their parent, AND processed as standalone files, and when processed as standalone files, the heading levels naturally appeared to be out of sequence. In the new functionality, the include:: is removed and the included text is appended directly to the parent file, which means that those files will no longer be double-processed as standalone files by jekyll.

HOWEVER, this does mean that if anyone has any bookmarked links to those formerly-standalone pages, those links will now be broken, since that content now only exists in the main flow.

I spot-checked a variety of things, like TOC contents and links in the text, and everything seemed to still be ok, but it's totally possible I overlooked something.

@nelliemckesson nelliemckesson removed the waiting for revisions Waiting for the OP to make revisions label Sep 29, 2023
@aallan
Copy link
Contributor

aallan commented Oct 1, 2023

HOWEVER, this does mean that if anyone has any bookmarked links to those formerly-standalone pages, those links will now be broken, since that content now only exists in the main flow.

I think this is just fine.

@aallan aallan added the ready to merge The OP says this PR is ready to merge? Anyone object? label Oct 1, 2023
@aallan
Copy link
Contributor

aallan commented Oct 1, 2023

Unless @lurch has a problem with this on Monday morning, I'm going to go ahead and merge.

@lurch
Copy link
Contributor

lurch commented Oct 2, 2023

I just spotted that when testing the output of this PR, the example-links on the Examples page now don't auto-scroll to the right place? I.e. on https://www.raspberrypi.com/documentation/pico-sdk/examples_page.html if I click on e.g. "UART example" it goes to https://www.raspberrypi.com/documentation/pico-sdk/hardware.html#uart_example and then scrolls to the right part of the page. But on http://127.0.0.1:4000/documentation/pico-sdk/examples_page.html if I click "UART example" it goes to http://127.0.0.1:4000/documentation/pico-sdk/hardware.html#uart_example but then I'm left looking at the "Hardware APIs" header at the very top of the page.

@aallan aallan added waiting for revisions Waiting for the OP to make revisions and removed ready to merge The OP says this PR is ready to merge? Anyone object? labels Oct 2, 2023
@lurch
Copy link
Contributor

lurch commented Oct 2, 2023

And I don't know if it's "expected" or "correct", but part of https://www.raspberrypi.com/documentation/pico-sdk/networking.html#cyw43_ll looks different:
Screenshot from 2023-10-02 11-55-47

when generated from this PR:
Screenshot from 2023-10-02 11-56-09

@nelliemckesson
Copy link
Contributor Author

@lurch I'm not seeing the reordered content -- here's what I see when I build and serve:

Screen Shot 2023-10-11 at 10 10 03 AM

@nelliemckesson
Copy link
Contributor Author

I'm also not experiencing the link/scrolling issue. Might make sense to just sit down together next week and compare what we're seeing, and see if we can figure out why it's different.

@aallan aallan added paused Waiting for other things to happen. and removed waiting for revisions Waiting for the OP to make revisions labels Oct 11, 2023
@aallan
Copy link
Contributor

aallan commented Oct 11, 2023

Might make sense to just sit down together next week and compare what we're seeing, and see if we can figure out why it's different.

Sounds like a good plan.

@lurch
Copy link
Contributor

lurch commented Oct 11, 2023

It might be that pesky "different versions of Doxygen produce different output" chestnut again? 🤷

@lurch
Copy link
Contributor

lurch commented Oct 17, 2023

I think the "weirdness" I was seeing in my local build was purely due to me using an old version (1.8.13) of Doxygen (due to me still running an old version of Ubuntu on my laptop). Works okay on Nellie's laptop (with Doxygen 1.9.7) so on that basis I'm happy to approve this PR. Sorry for holding things up!
(I checked, and our CI build uses Doxygen 1.9.6 )

@aallan aallan merged commit 6f6318c into develop Oct 17, 2023
1 check passed
@aallan aallan deleted the iss-2828 branch October 17, 2023 14:08
@lurch
Copy link
Contributor

lurch commented Oct 17, 2023

HOWEVER, this does mean that if anyone has any bookmarked links to those formerly-standalone pages, those links will now be broken, since that content now only exists in the main flow.

@nelliemckesson Presumably there'll never have been any links (from within our docs) to these standalone-pages, so users would only have been able to find these URLs by guessing at filenames?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix paused Waiting for other things to happen. toolchain This is an infrastructure/toolchain issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Doxygen: Out of sequence warnings
3 participants