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.

Sign up for GitHub

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

Jump to bottom

feat(ApplicationCommands): attachment application command option type #7123

Closed
wants to merge 12 commits into from
Closed

feat(ApplicationCommands): attachment application command option type #7123

wants to merge 12 commits into from

Conversation

amitojsingh366
Copy link
Contributor

@amitojsingh366 amitojsingh366 commented Dec 18, 2021 •
edited

Please describe the changes this PR makes and why it should be merged:
Adds the new ATTACHMENT command option type and also adds patching for filename in MessageAttachment#_patch

Status and versioning classification:
This PR relies on This PR relies on discordjs/discord-api-types#272 and discord/discord-api-docs#4253

ImRodry
ImRodry suggested changes Dec 18, 2021
View reviewed changes
Copy link
Contributor

@ImRodry ImRodry left a comment

Choose a reason for hiding this comment

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

You forgot to add the new option to ApplicationCommand#_optionEquals, #transformOption, ApplicationCommandOption, ApplicationCommandOptionData, Constants#ApplicationCommandOptionType, and probably some typings although I'm not sure on that one

src/structures/CommandInteractionOptionResolver.js Outdated Show resolved Hide resolved
@amitojsingh366 amitojsingh366 changed the title feat(ApplicationCommands): attachment application command option type WIP: feat(ApplicationCommands): attachment application command option type Dec 18, 2021
@amitojsingh366 amitojsingh366 marked this pull request as draft December 18, 2021 01:42
@amitojsingh366
Copy link
Contributor Author

You forgot to add the new option to ApplicationCommand#_optionEquals, #transformOption, ApplicationCommandOption, ApplicationCommandOptionData, Constants#ApplicationCommandOptionType, and probably some typings although I'm not sure on that one

I have marked this pr as a draft for now since I am still unsure about the Interaction structure, I will update this PR soon

@amitojsingh366
Copy link
Contributor Author

@ImRodry could you have a look at this PR again, I figured the data structure out and modified this PR accordingly.
You can find the data structure and how I found it in this gist

@amitojsingh366 amitojsingh366 marked this pull request as ready for review December 19, 2021 00:06
@ImRodry
Copy link
Contributor

ImRodry commented Dec 19, 2021

Please add discord/discord-api-docs#4253 to the PR description btw

@amitojsingh366
Copy link
Contributor Author

Please add discord/discord-api-docs#4253 to the PR description btw

done!

ImRodry
ImRodry suggested changes Dec 19, 2021
View reviewed changes
src/structures/BaseCommandInteraction.js Outdated Show resolved Hide resolved
src/structures/BaseCommandInteraction.js Outdated Show resolved Hide resolved
src/structures/CommandInteractionOptionResolver.js Outdated Show resolved Hide resolved
src/structures/MessageAttachment.js Outdated Show resolved Hide resolved
src/util/Constants.js Show resolved Hide resolved
@amitojsingh366 amitojsingh366 changed the title WIP: feat(ApplicationCommands): attachment application command option type feat(ApplicationCommands): attachment application command option type Dec 19, 2021
@iCrawl iCrawl added this to the Version 13.x milestone Dec 21, 2021
vladfrangu
vladfrangu previously requested changes Dec 21, 2021
View reviewed changes
src/structures/BaseCommandInteraction.js Outdated Show resolved Hide resolved
src/structures/BaseCommandInteraction.js Outdated Show resolved Hide resolved
vladfrangu
vladfrangu previously requested changes Dec 23, 2021
View reviewed changes
typings/index.d.ts Outdated Show resolved Hide resolved
@iCrawl iCrawl modified the milestones: Version 13.x, Version 14 Dec 24, 2021
@SocketSomeone SocketSomeone mentioned this pull request Dec 29, 2021
Closed
1 task
@amitojsingh366
Copy link
Contributor Author

closing this in favor of #7200

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ImRodry ImRodry ImRodry requested changes

@NotSugden NotSugden NotSugden requested changes

@kyranet kyranet Awaiting requested review from kyranet

@SpaceEEC SpaceEEC Awaiting requested review from SpaceEEC

@vladfrangu vladfrangu Awaiting requested review from vladfrangu

Assignees
No one assigned
Labels
api support discord interactions packages:discord.js semver:minor
Projects
discord.js
Archived in project
Milestone
discord.js v14
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@amitojsingh366 @ImRodry @vladfrangu @NotSugden @iCrawl