-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Add strategy to identify Roff man pages: Take 2 #4433
Conversation
This pull request has been automatically marked as stale because it has not had recent activity, and will be closed if no further activity occurs. If this pull request was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions. |
pls no close stalebot, I'll try and find a reviewer |
This pull request has been automatically marked as stale because it has not had recent activity, and will be closed if no further activity occurs. If this pull request was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions. |
any update? :-) |
This pull request has been automatically marked as stale because it has not had recent activity, and will be closed if no further activity occurs. If this pull request was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions. |
Nudging @lildude for input. |
Whoops, completely missed that. 😀 Fixed! |
Nudging @pchaigno. |
This pull request has been automatically marked as stale because it has not had recent activity, and will be closed if no further activity occurs. If this pull request was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions. |
If somebody — anybody — could review this, shut stalebot up, and make many man-page lovers happy, that'd be seriously copacetic. Seriously. /cc @pchaigno again |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the current regular expression is going to create issue with other languages (see below). I'm not sure how to best handle this. It's probably better if we avoid hardcoding the problematic extensions...
Well, that can always be solved by generating a But in any case, this shouldn't be a problem since the manpage-strategy has a lower priority than the filename one does, so "problematic" extensions shouldn't be a problem. 😉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Merci! 😀 @lildude Alright to merge? ^^ |
/cc'ing @lildude in case he's missed this. |
Yessssssssssss |
@jordemort Any update on github/markup#1196? |
This PR adds a strategy to identify man-pages with unrecognised file extensions. Added in support of github/markup#1196.
Description
This was previously attempted in #4317, but ditched after the complexity of the PR convinced me a simpler solution was needed to safeguard against incorrectly rendered man-pages. That concern has since been addressed by #4393, leaving us free to adopt a much simpler implementation.
Note that the
Manpage
strategy deliberately returns two language entries, deferring the exact classification to the heuristics (which determine whether a Roff file should be rendered as a man-page or not).References: #4258, #4309, #4317
/cc @pchaigno, @lildude, @jordemort, @pali
Checklist: