Skip to content
This repository was archived by the owner on Feb 21, 2024. It is now read-only.

Add filter struct - #42

Merged
erikjohnston merged 1 commit into
matrix-org:masterfrom
jcgruenhage:master
Oct 3, 2017
Merged

erikjohnston merged 1 commit into
matrix-org:masterfrom
jcgruenhage:master

Conversation

@jcgruenhage

Copy link
Copy Markdown
Contributor

No description provided.

@jcgruenhage jcgruenhage changed the title Add filter struct WIP: Add filter struct Sep 29, 2017
@jcgruenhage jcgruenhage changed the title WIP: Add filter struct Add filter struct Sep 29, 2017
Comment thread filter.go Outdated

type Filter struct {
AccountData struct {
Limit int `json:"limit"`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe this should be a * int so we can handle 0 more intelligently?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How does this change handling of the 0?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If limit is unspecified then int will default to 0 while *int will default to nil, this would then allow us (elsewhere) to differentiate between the two cases.

Specifying a limit of 0 feels a bit weird, but it should probably not just mean use the default limit, which is what would happen if we use int here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, that makes sense, yes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This should apply to all limits in this struct, doesn't it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yup

Comment thread filter.go Outdated
NotSenders []string `json:"not_senders,omitempty"`
NotTypes []string `json:"not_types,omitempty"`
Senders []string `json:"senders,omitempty"`
Types []string `json:"types,omitempty"`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This struct comes up a lot, should it not be pulled up into a separate thing? I think in synapse I called the top most struct FilterCollection and then Filter for the sub struct

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah sure... If I had written it all by hand, I would have probably noticed that, I generated the struct from the json schema 😄

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Haha, fair. I'm imaging that at some point we may want to write functions that take the sub filters as an argument, instead of manually writing the filtering code for each of account data, ephemeral, events, etc

@erikjohnston
erikjohnston merged commit a7fc80c into matrix-org:master Oct 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants