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

Message#edit shouldn't remove content #3129

Merged
merged 4 commits into from Mar 8, 2019

Conversation

Projects
None yet
3 participants
@bdistin
Copy link
Member

bdistin commented Mar 5, 2019

Please describe the changes this PR makes and why it should be merged:
This fixes an inconsistency with the api. If you patch only a data embed on a message, the content will be maintained. This allows Message#edit to match that behavior.

const msg = await message.channel.send('test');
const edit = await msg.edit({ embed: { description: 'more tests' } });
return edit.content;

Before this pr the content would be '', after this pr the content will be 'test'.

Please note this also maintains the consistency for passing { content: null } which will remove the content, just as { embed: null } would remove the embed.

Status

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating

Semantic versioning classification:

  • This PR changes the library's interface (methods or parameters added)
    • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
  • This PR only includes non-code changes, like changes to documentation, README, etc.

Although this has breaking behavior in terms of the current library expectations, this should technically be considered a bugfix.

bdistin added some commits Mar 5, 2019

edit shouldn't remove content
If undefined is passed to the api, content isn't removed in such a case where you are only editing the embed.
@kyranet

kyranet approved these changes Mar 6, 2019

@SpaceEEC
Copy link
Member

SpaceEEC left a comment

Otherwise LGTM.

Show resolved Hide resolved typings/index.d.ts Outdated

@SpaceEEC SpaceEEC merged commit 1673b6f into discordjs:master Mar 8, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

Koyamie added a commit to Koyamie/discord.js that referenced this pull request Mar 11, 2019

Merge pull request #33 from discordjs/master
fix(APIMessage): edit shouldn't remove content (discordjs#3129)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.