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

bin/lesson_check.py: allow exceptions to line length limit #594

Merged

Conversation

maxim-belkin
Copy link
Contributor

Allow lines that contain a single image or a single link to go over the suggested line length limit. So, all of the following lines will be allowed to go over the line limit:

[alt text](/some/url)
![alt text](/path/to/image)
[alt text][internal_link]
![alt text][internal_link_to_image]

Plus all of the above prefixed with any combination of > and to allow these lines to go over the limit in various code blocks.

Fixes #591

Allow lines that contain a single image or a single link
to go over the suggested line length limit.
@maxim-belkin maxim-belkin force-pushed the link-image-only-lines branch from ab46307 to 0f1cec8 Compare May 6, 2021 17:18
@fmichonneau
Copy link
Contributor

Thanks Maxim!

@maxim-belkin maxim-belkin merged commit c82a669 into carpentries:gh-pages May 6, 2021
@maxim-belkin maxim-belkin deleted the link-image-only-lines branch May 6, 2021 19:14
@maxim-belkin
Copy link
Contributor Author

Thanks, François!

@jhlegarreta, could you please confirm that after this change make lesson-check-all doesn't complain about the long lines that contain images or links only?

@jhlegarreta
Copy link

The regex does not seem straightforward ! Thanks both 👍.

@jhlegarreta, could you please confirm that after this change make lesson-check-all doesn't complain about the long lines that contain images or links only?

Will give it a try when I find the time (will try during the weekend) and will report back.

@jhlegarreta
Copy link

jhlegarreta commented May 7, 2021

Unfortunately, it does not look like it solved the issue. In fact, the number of warnings has increased, e.g.:
https://github.com/carpentries-incubator/SDC-BIDS-dMRI/runs/2531577900?check_suite_focus=true#step:16:20
with respect to an immediately previous version, e.g.
https://github.com/carpentries-incubator/SDC-BIDS-dMRI/runs/2528767166?check_suite_focus=true#step:16:20

In the previous version, link-only lines (e.g. line 190 at the pointed file - __episodes/constrained_spherical_deconvolution.md) were not triggering warnings, and now they are. Meanwhile, the lines that made me open (e.g. line 458) the issue are still raising the warnings.

@maxim-belkin
Copy link
Contributor Author

Oh, I see what the problem is. The pattern I added doesn't allow {:class ...}at the end. I'll fix that.

@maxim-belkin
Copy link
Contributor Author

maxim-belkin commented May 9, 2021

So, I found a few things that you use in your lesson that I didn't take into account when I was working on this pattern:

  1. You use links in headings
  2. You use {:class=...} customizations in link/image-only lines
  3. You use \ at the end of the image/link-only lines

So, #597 should address all of these.

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.

Allow skipping line length checks for link-only lines
3 participants