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

WikilogUtils: Replace WikiPage::getText() with getContent() #21

Merged

Conversation

refeed
Copy link
Contributor

@refeed refeed commented Dec 23, 2017

Replace WikiPage::getText() with
WikiPage::getContent()->getNativeData() in
WikilogUtils.php, since WikiPage::getText() had been removed
from WikiPage.

Bug: T179532

WikilogUtils.php Outdated
@@ -210,7 +210,7 @@ public static function parsedArticle( Title $title, $feed = false ) {
$parser->startExternalParse( $title, $parserOpt, Parser::OT_HTML );

# Parse article.
$arttext = $article->getText();
$arttext = $article->getContent()->getNativeData();

Choose a reason for hiding this comment

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

Please check, if the returned Content object is a TextContent (instanceof TextContent), as this is currently the implementation, which will return text for getNativeData(). Any other implementation will vary depending on the content model of the page.

Here in this case, I think it's best to just check if the content model is TextContent, and if not return default values (null or empty things).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay!

WikilogUtils.php Outdated
@@ -328,7 +328,7 @@ public static function categoryList( $list ) {
foreach ( $list as $cat ) {
$title = Title::makeTitle( NS_CATEGORY, $cat );
$categoryUrl = $title->getPrefixedText();
$categoryTxt = $title->getText();
$categoryTxt = $title->getContent()->getNativeData();

Choose a reason for hiding this comment

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

same as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry my bad, I think $title here is not related with WikiPage

WikilogUtils.php Outdated
@@ -365,7 +365,7 @@ public static function tagList( $list ) {
public static function splitSummaryContent( $parserOutput ) {
global $wgUseTidy;

$content = Sanitizer::removeHTMLcomments( $parserOutput->getText() );
$content = Sanitizer::removeHTMLcomments( $parserOutput->getContent()->getNativeData() );

Choose a reason for hiding this comment

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

same as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think $title here is not related with WikiPage

@refeed refeed force-pushed the fix_deprecated_gettext_method branch from 01f407b to 11b442b Compare December 23, 2017 16:04
WikilogUtils.php Outdated
@@ -210,7 +210,8 @@ public static function parsedArticle( Title $title, $feed = false ) {
$parser->startExternalParse( $title, $parserOpt, Parser::OT_HTML );

# Parse article.
$arttext = $article->getText();
$articleContent = $article->getContent();
$arttext = $articleContent instanceof TextContent ? $articleContent->getNativeData() : false;

Choose a reason for hiding this comment

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

Is false really a good default value? What do you think about checking, if the content is a TextContent, and if not, returning with an empty string as the text, as it would be pointless to proceed in the codepath with an empty string I think.

return array( $article, '');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay

Replace `WikiPage::getText()` with
`WikiPage::getContent()->getNativeData()` in
`WikilogUtils.php`, since `WikiPage::getText()` had been removed
from `WikiPage`.

Bug: T179532
@refeed refeed force-pushed the fix_deprecated_gettext_method branch from 11b442b to 97d1e06 Compare December 23, 2017 16:43
$arttext = $article->getText();
$articleContent = $article->getContent();
if ( !($articleContent instanceof TextContent) ){
# Restore default behavior.
Copy link
Contributor Author

@refeed refeed Dec 23, 2017

Choose a reason for hiding this comment

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

From line 230, I think we need to restore the default behavior (I think it's related to the parser).

	/**
	 * Enable special wikilog feed parsing.
	 *
	 * This function changes the parser behavior in order to output
	 *
	 * The proper way to use this function is:
	 * @code
	 *   $saveFeedParse = WikilogParser::enableFeedParsing();
	 *   # ...code that uses $wgParser in order to parse articles...
	 *   WikilogParser::enableFeedParsing( $saveFeedParse );
	 * @endcode
	 *
	 * @note Using this function changes the behavior of Parser. When enabled,
	 *   parsed content should be cached under a different key.
	 */
	public static function enableFeedParsing( $enable = true ) {
		$prev = self::$feedParsing;
		self::$feedParsing = $enable;
		return $prev;
	}

@vitalif vitalif merged commit 8f2df05 into mediawiki4intranet:master Dec 27, 2017
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.

None yet

3 participants