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

Add ChannelEditComplex #493

Merged
merged 9 commits into from
Dec 15, 2017
Merged

Add ChannelEditComplex #493

merged 9 commits into from
Dec 15, 2017

Conversation

psheets
Copy link
Contributor

@psheets psheets commented Dec 14, 2017

Add ChannelEditComplex function and ChannelEdit struct.

Allows updating a channel's fields such as topic.

Implemented as suggested by @iopred in #322 and #492

@Seklfreak
Copy link
Contributor

I think all possible attributes should be added. ( https://discordapp.com/developers/docs/resources/channel#modify-channel )

Missing right now: position, user_limit, permission_overwrites and parent_id.

@psheets
Copy link
Contributor Author

psheets commented Dec 14, 2017

ChannelEdit struct has been updated to include all fields per @Seklfreak comment.

@iopred
Copy link
Collaborator

iopred commented Dec 14, 2017

2 things:

  1. The file encodings changed again
  2. Modify the existing ChannelEdit method to call the new ChannelEditComplex method

Thank you so much for this PR!

@Strum355
Copy link
Contributor

Strum355 commented Dec 14, 2017

Re. point 1, I havent heard them being called "file encodings", so for those who havent heard this, its the files permissions (as per *nix) that have been changed from 644 to 755. On that note, the image most definitely shouldnt need to be 755

@iopred
Copy link
Collaborator

iopred commented Dec 14, 2017

Ah, sorry I didn't look closely, I've also seen this when the newline character (\n -> \r\n) changes, which is why I said encoding.

@Strum355
Copy link
Contributor

Strum355 commented Dec 14, 2017

I ran the files and its current live one through dos2unix and cmp, no difference in encoding, so it must be just a permissions thing ^^ Still no need for the change in permissions, so that should be reverted, yea

@psheets
Copy link
Contributor Author

psheets commented Dec 15, 2017

Reverted permissions back to 644 and Modified ChannelEdit to use ChannelEditComplex per @iopred and @Strum355

restapi.go Outdated

body, err := s.RequestWithBucketID("PATCH", EndpointChannel(channelID), data, EndpointChannel(channelID))
// ChannelEditComplex edits an existing channel, replacing the parameters entirely with ChannelEdit struct
func (s *Session) ChannelEditComplex(c *ChannelEdit) (st *Channel, err error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry to be a pain, could we change this method signature to:

ChannelEditComplex(channel ID string, c *ChannelEdit)

We can then remove the ID field from the struct.

This would bring this API closer to the ChannelMessageSendComplex method.

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a pain, thank you for the feedback. This is a learning experience for me.

The current signature is mimicking ChannelMessageEditComplex:
func (s *Session) ChannelMessageEditComplex(m *MessageEdit) (st *Message, err error)

and the MessageEdit struct:

type MessageEdit struct {
	Content *string       `json:"content,omitempty"`
	Embed   *MessageEmbed `json:"embed,omitempty"`

	ID      string
	Channel string
}

Should ChannelMessageEditComplex be rewritten to be inline with your suggestion?
Adding channelID seems to match the other functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see that changing ChannelMessageEditComplex will break the existing API.

@iopred iopred merged commit e024d5f into bwmarrin:develop Dec 15, 2017
@iopred
Copy link
Collaborator

iopred commented Dec 15, 2017

Thanks so much!

@bwmarrin bwmarrin added this to the v0.18.0 milestone Dec 17, 2017
ErikMcClure pushed a commit to ErikMcClure/discordgo that referenced this pull request Aug 4, 2020
* Add ChannelEditComplex

* Fixed comment format

* gofmt

* Reverted permissions and fixed ChannelEditComplex

* Reverted Perms

* Delete discordgo - Shortcut.lnk

removed link

* Added ChannelID param to ChannelEditComplex

* gofmt
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

5 participants