happy.melon.wiki wrote:

If you want to write a patch for this, I'll review and commit it. My workload is escalating up to the last week in June, however, so I may become less available as time goes by.

tisane2718 wrote:

OK, I'll start work on that. This will probably be done as part of the same patch that fixes bug 23655.

tisane2718 wrote:

Patch to Article.php fixing this bug

This changes $watchthis to &$watchthis so that hooks (ArticleSave, ArticleInsertComplete, and ArticleSaveComplete) can change its value. It also introduces $alterWatchlist, which if null causes $watchthis to be ignored. The alterations to the watchlist are now done by doEdit rather than by insertNewArticle or updateArticle, which I guess moves us a step closer to deprecating those two functions.

attachment patch_file ignored as obsolete

tisane2718 wrote:

Actually, I think I just figured out how to fix bug 23655, so I'll be submitting a substitute patch to fix both.

happy.melon.wiki wrote:

• $this->doEdit( $text, $summary, $flags );

+ $this->doEdit( $text, $summary, $flags, $false, null, $watchthis, true );

"$false"?? Please test your code before submitting.

+ * @param $watchthis Unwatch the page if null, otherwise watch it; ignored if $alterWatchlist is null
+ * @param $alterWatchlist If null, watchlist will be left alone; otherwise page will be watched or unwatched,
+ * depending upon value of $watchthis

It would be nicer to implement this as one parameter $watch, where the page is watched if true, unwatched if false, and left unchanged if null. Right now there are no hooks in SVN making use of the existing $watchthis parameter, so there's no harm in slightly tweaking its implementation.

tisane2718 wrote:

Patch to Article.php fixing this bug and bug 23655

This patch moves most of the functionality of insertNewArticle and updateArticle into doEdit, which should make it pretty easy to modify the rest of the core so as to deprecate those former two functions (although that'll be another patch). &$redirect is introduced, which allows the hook function to choose whether or not the user will be redirected back to the page after the edit has been successfully committed.

attachment patch_file_2 ignored as obsolete

tisane2718 wrote:

I can fix this patch to implement your suggestions from comment 5, or wait till you've reviewed the second patch and then create a third patch fixing whatever additional problems you may point out; whichever you prefer.

tisane2718 wrote:

Fix items from message #5.

Attached:

tisane2718 wrote:

I wonder if I am taking the right approach in folding most of the code from the other functions into doEdit, or whether it would be a better idea to fold it out into the functions that call insertNewArticle and updateArticle?

happy.melon.wiki wrote:

If your changes reduce code duplication, which they seem to do; then you probably have the right approach. It's pretty rare that increasing duplication is the right thing to do.

happy.melon.wiki wrote:

Patch applied in r66878. Does this also fix bug 23655?

tisane2718 wrote:

Yes, it does. Thanks for your help.

Reverted r66878.

Add another hook, don't use ArticleSaveComplete for this.