refactor(Embed): remove add field

[imranbarbhuiya]

Please describe the changes this PR makes and why it should be merged:
This PR removes Embed#addField in favour of Embed#addFields because you are just saving a s when using addField.

Status and versioning classification:

• 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
• This PR changes the library's interface (methods or parameters added)
• This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

For those who don't wanna replace all addField manually can try this regex replace. You can ofc make a better one but using these 2 also works fine. Idk how can I use condition in replace string else it can be simplified to one regex using ? after inline.

// 1. use this one first
// regex
.addField\(((.|[\n])*?),((.|[\n])*?),(.*true)\)
//replace with
.addFields({name: $1, value: $3, inline: $5})

// 2. use this one after replacing all inline fields
//regex
.addField\(((.|[\n])*?),((.|[\n])*?)\)
//replace with
.addFields({name: $1, value: $3})

cross verify before replacing.

[ImRodry]

I think this method should be changed to match v13’s MessageEmbed#addField instead because it is quite useless as it is right now. I don’t think removing it is the way to go because there are many situations where you may only need one field and creating the object just adds unnecessary visual burden to the code

[vladfrangu]

I think the method should stay as is, although I can see how it's redundant compared to addFields... 🤷 I'll let the others give in their thoughts too

[nev-r]

i think it's nicer if d.js is ugly so that downstream code can look clean.
allocating a new array and wrapping it in extra syntax to add a single field

addFields([field])

just to make the method happy, is kind of a tragedy.

i'd personally rather recommend people spread into addField than the above mess

[vladfrangu]

That's not how the spread arguments work, you don't need to wrap them in brackets

[nev-r]

i didn't say they need to be in brackets?
i said i would rather do addField(...myfields) or addFields(...myfields)
than be forced to to addFields([myfield])
(if the goal is to narrow to 1 method)

[BenjammingKirby]

why just remove addField and not other builder methods like addChoice an such?

[BenjammingKirby]

i didn't say they need to be in brackets?
i said i would rather do addField(...myfields) or addFields(...myfields)
than be forced to to addFields([myfield])
(if the goal is to narrow to 1 method)

the methods exclusively take a rest parameter since #7395

[nev-r]

the methods exclusively take a rest parameter since #7395

ah, thanks. then yeah, such a little stub passthrough function like addField seems unnecessary

[suneettipirneni]

why just remove addField and not other builder methods like addChoice an such?

If this goes through they should be removed.

[BenjammingKirby]

I think this method should be changed to match v13’s MessageEmbed#addField instead because it is quite useless as it is right now. I don’t think removing it is the way to go because there are many situations where you may only need one field and creating the object just adds unnecessary visual burden to the code

you are forgetting why the change was made
changing to objects was the right call since some of the methods may take multiple optional properties, like setAuthor
what do you want? setAuthor(something, undefined, something)? an object is much better
and with this change they are now useless

[ImRodry]

That argument makes no sense because fields are composed of 2 required parameters and 1 optional one

[BenjammingKirby]

That argument makes no sense because fields are composed of 2 required parameters and 1 optional one

i never said this particular one does, but i did give you an example didn't i?
so what do you suggest? keep it like this for just this one method but for the others use objects? that to me makes no sense, we should try to stay consistent

[ImRodry]

The example you gave is invalid because it comes from a situation with many optional parameters, which this one isn’t and you’d never be passing undefined for the first or second values because that would be an error

[advaith1]

If the goal is to remove redundancy then imo addFields should be removed instead of addField, which is probably used a lot more.

[almostSouji]

Semantically, addFields makes much more sense, you can add multiple, but adding one is fine too. Whereas adding multiple to addField is not nearly as intuitive

[suneettipirneni]

If the goal is to remove redundancy then imo addFields should be removed instead of addField, which is probably used a lot more.

Considering what's used more it's kinda hard to tell I wouldn't say for certain that one is used a lot more than the other. However, grammatically it makes more sense for it be plural since it accepts a rest parameter which is 1..n.