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

Fix editing section zero #249

Closed
wants to merge 1 commit into from
Closed

Fix editing section zero #249

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 27, 2020

Currently, Page.text() correctly allows retrieving only section 0. However, Page.edit() (internally, Page._edit()) treats 0 as a falsy value and fails to set the section key in the data object sent to the API, meaning it's very feasible for users to retrieve section 0 and accidentally overwrite the entire page with only its contents.

This PR changes the comparison to the more explicit is not None used elsewhere.

@ghost ghost changed the title Fix editing section zero - Apr 6, 2020
@ghost ghost closed this Apr 6, 2020
@ghost ghost deleted the edit-section-zero branch April 6, 2020 09:41
@waldyrious
Copy link
Member

@1bakedpotato I hope you don't mind, but I'll restore the original title and description of this PR. Otherwise it would look quite puzzling in the issue tracker.

@waldyrious waldyrious changed the title - Fix editing section zero Apr 6, 2020
@waldyrious
Copy link
Member

@danmichaelo is this something we should merge? I haven't touched this code in a while so I'm not sure.

danmichaelo added a commit that referenced this pull request May 28, 2020
Initially proposed at #249
@danmichaelo
Copy link
Member

danmichaelo commented May 28, 2020

Yes, we should! Sorry I've not been very responsive for some time. Since the fork was deleted, I wasn't able to fetch the commit from @ghost anymore, but I will give credit in the changelog! eh, no, I can't, since the user is deleted as well 😮 Oh well, at least a reference to the PR then :/

@waldyrious
Copy link
Member

@danmichaelo actually it is still possible to check out the pull request; GitHub keeps a copy of the branch stored in the target repository, which you can checkout locally via git fetch origin pull/PR_NUMBER/head:BRANCH_NAME; in this case, that would be:

git fetch origin pull/249/head:edit-section-zero

That's explained in this GitHub help article. Hope that helps for next time :)

@danmichaelo
Copy link
Member

danmichaelo commented May 28, 2020

Hah, thanks, that's nice to know :) At least, I added credits to 1bakedpotato to https://github.com/mwclient/mwclient/releases/tag/v0.10.1

This pull request was closed.
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

2 participants