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

[MISC] Added appendix to mkdocs and added some internal links #77

Merged
merged 27 commits into from
Oct 31, 2018

Conversation

franklin-feingold
Copy link
Collaborator

  • Added appendix to mkdocs
  • Added several internal links (Add in internal links #8)
  • Edited CHANGE for further automation (no linter errors) and added internal links

CHANGES.md Outdated
- Added DwellTime field.
- Added support for MEG data (merged BEP008)
- Added SequenceName field.
- Added support for describing events with Hierarchical Event Descriptors [[4.3 Task events](https://bids-specification.readthedocs.io/en/latest/04-modality-specific-files/03-task-events.html)].
Copy link
Collaborator

Choose a reason for hiding this comment

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

isn't there a way to provide internal references so they work correctly in both html and pdf renderings? (I am not yet familiar with this doc building setup so do not have an answer ready myself)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can provide a relative path linking than including the URL. Readthedocs can handle that. I confirmed this implementation can work with the HTML rendering, but pdf rendering appears to still be an open issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

update - I went through and changed the linking to the actual markdown files on github. Hopefully that should stabilize the references

Copy link
Collaborator

Choose a reason for hiding this comment

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

cool! I did the same in my PR: https://github.com/bids-standard/bids-specification/pull/78/files#diff-d668661006216379ab2a5c5ff7061b8aR221

my concern though is the "fragility" of #id anchors. If misspecified/renamed file would be detected by mkdocs, those #id could be mistyped and "the system" seems to not detect it :-( dunno yet what (if anything) to do about it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice! In respect to the linking, I think most of the filenames appear to be solid. For the linking I think it will be prone to some fragility because we still need to link to something. When I am building locally, mkdocs does give a warning if it sees a link that is not present (or if I gave an incorrect path to the file), perhaps giving a test to make sure that no warning exists? Though, I can't either think of a more robust solution that would be linking onto something more concrete (than the filenames).

Copy link
Collaborator

Choose a reason for hiding this comment

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

just for clarity: yes, it issues the warning if the file is misspecified, but it would not warn if the portion after # is incorrect. E.g. if you link to 04-modality-specific-files/01-magnetic-resonance-imaging-data.md#sequence-specificsTYPO it wouldn't warn that there is incorrect sequence-specificsTYPO.
Indeed I do not thin we could achieve a better solution (besides adding some post-build ad-hoc testing) with this doc system -- just for comparison in sphinx references are more explicit, so build system would warn/fail if :ref: (possibly into within a subsection in a document) is wrong

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see what you are saying. Yes the best solution I can think of too is the ad-hoc testing. I suppose for now with this system is to be careful regarding referencing subsections within a file and checking locally that the links work.

@teonbrooks
Copy link
Collaborator

looks like this needs to be rebase as there are redundant commits in the history

@franklin-feingold
Copy link
Collaborator Author

what are the redundant commits you are referencing? rebasing my old_contrib?

@teonbrooks
Copy link
Collaborator

I think it includes the commits from #44, which may not be ideal, instead of having the master branch as base.

@chrisgorgo chrisgorgo merged commit b519a65 into bids-standard:master Oct 31, 2018
@franklin-feingold franklin-feingold deleted the old_contrib branch October 31, 2018 21:18
@sappelhoff sappelhoff changed the title Added appendix to mkdocs and added some internal links [INFRA] Added appendix to mkdocs and added some internal links Sep 8, 2020
@sappelhoff sappelhoff changed the title [INFRA] Added appendix to mkdocs and added some internal links [MISC] Added appendix to mkdocs and added some internal links Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants