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 support for handling Attachment and Role objects. #44

Closed
wants to merge 25 commits into from

Conversation

clin1234
Copy link
Contributor

@clin1234 clin1234 commented Jun 8, 2021

No description provided.

@guerinoni guerinoni self-requested a review June 8, 2021 19:00
Copy link
Collaborator

@guerinoni guerinoni left a comment

Choose a reason for hiding this comment

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

Please clean git history with worth commit messages and indent using clang-format (based on our configuration)

src/DiscordApi/DiscordAPI.hpp Show resolved Hide resolved
@@ -3,6 +3,7 @@
#include "core/Types.hpp"

#include <QNetworkAccessManager>
#include <utility>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Header where std::pair is declared

src/core/User.hpp Show resolved Hide resolved
@@ -5,8 +5,10 @@

#include <QVector>
#include <QtDebug>
#include <optional>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not used

Copy link
Contributor Author

@clin1234 clin1234 Jun 9, 2021

Choose a reason for hiding this comment

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

I disagree. Given the numerous fields that could not exist when a user object is returned, a using statement is necessary.

src/core/User.cpp Show resolved Hide resolved
src/core/Attachment.hpp Show resolved Hide resolved
#include <QJsonObject>
#include <optional>

using std::optional;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove using

void Attachment::unmarshall(const QJsonObject &obj)
{
m_id = obj["id"].toString().toULongLong();
// FIXME: tell Qt to add a convenience function to convert to size_t
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is a Qt based software we should have a trade off between c++ std and Qt policy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given the nature of manipulating files in C++, I assume that the maximum file size of an attachment from Discord is size_t (not guaranteed to be synonymous with unsigned long long or even exactly 64 bits long.

Comment on lines 20 to 33
/*
bool key_exists(const QJsonObject &k, QString &l)
{
<<<<<<< HEAD
return !k[l).isNull();
=======
return !k.value(l).isNull();
<<<<<<< HEAD
>>>>>>> 4ec4a79 (WIP)
=======
>>>>>>> 2d0b1ee5ce0a55777d3de9e1449fa0f809496b37
>>>>>>> 17a0b43a7993c64631a46f28011ba408220b3e08
}*/

Copy link
Collaborator

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some screwup with Git regarding a unused function. Ignore it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be removed

@@ -4,17 +4,20 @@
#include "Types.hpp"

#include <QJsonObject>
#include <optional>

using std::optional;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove using

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 disagree. Given the numerous fields that could not exist when a message object is returned, a using statement is necessary.

@clin1234 clin1234 closed this Jun 15, 2021
clin1234 pushed a commit to clin1234/unofficial-discord-client that referenced this pull request Jun 27, 2021
# This is the 1st commit message:

Add preliminary support for attachments and roles.

# This is the commit message #2:

Use #pragma once for headers, and start part 1 in adding Embed objects.

# This is the commit message #3:

Add #pragma once to Embed.hpp.

# This is the commit message #4:

Rename Role files to proper case.

# This is the commit message #5:

Follow advice from clazy: make string arguments to Qt functions QStringLiterals.

# This is the commit message #6:

Add initial support for buttons ans custom emoji. Part 1.

# This is the commit message #7:

Part 2 of Component support.

# This is the commit message #8:

Apply temporary fix to compilation errors involving emoji handling.

# This is the commit message #9:

More clazy advice-following in Message.cpp

# This is the commit message #10:

Refactor guild handling to account for guild preview objects.

# This is the commit message #11:

Fix missing data member after rebase.

# This is the commit message #12:

Fix spurious rebase mistake.

# This is the commit message #13:

Fix rebase errors in channel files

# This is the commit message #14:

Add handling for voice and voice region objects.

Actual handling of voice connections is not yet supported.

# This is the commit message #15:

Fix source file inclusion, and more clazy fixes.

# This is the commit message #16:

Make corrections in accordance to Coding-Bunker#49.

# This is the commit message #17:

Add gateway files to prevent CMake errors about missing files.

# This is the commit message #18:

Add rudimentary MenuBar on main QML file.

# This is the commit message #19:

Fix rebase issue

# This is the commit message #20:

Run js-beautify

# This is the commit message #21:

Omit version number, add features for channel and message objects.

# This is the commit message #22:

Clang-format misformatted files due to Qt Creator shenaniganry.

# This is the commit message #23:

Add preliminary support for attachments and roles.

# This is the commit message #24:

Add preliminary support for attachments and roles.

# This is the commit message #25:

Replace .value call with subscript operator for settings.

# This is the commit message #26:

Remove Git diff crud left by a previous commit.

# This is the commit message #27:

Fix formatting issue introduced by Qt Creator.

# This is the commit message Coding-Bunker#28:

Run clang-format

# This is the commit message Coding-Bunker#29:

Fix up bungles of previous commits.

# This is the commit message Coding-Bunker#30:

Add preliminary support for attachments and roles.

# This is the commit message Coding-Bunker#31:

Add preliminary support for attachments and roles.

# This is the commit message Coding-Bunker#32:

Fix build errors, and rename role files to proper case.

# This is the commit message Coding-Bunker#33:

Clean code

Signed-off-by: Federico Guerinoni <[email protected]>

# This is the commit message Coding-Bunker#34:

Add latest version of API

Signed-off-by: Federico Guerinoni <[email protected]>

# This is the commit message Coding-Bunker#35:

Fix minor mistake arising from rebase.

# This is the commit message Coding-Bunker#36:

Run clang-format

# This is the commit message Coding-Bunker#37:

Omit version number, add features for channel and message objects.

# This is the commit message Coding-Bunker#38:

Add preliminary support for attachments and roles.

# This is the commit message Coding-Bunker#39:

Add preliminary support for attachments and roles.

# This is the commit message Coding-Bunker#40:

Replace .value call with subscript operator for settings.

# This is the commit message Coding-Bunker#41:

Remove Git diff crud left by a previous commit.

# This is the commit message Coding-Bunker#42:

Fix formatting issue introduced by Qt Creator.

# This is the commit message Coding-Bunker#43:

Run clang-format

# This is the commit message Coding-Bunker#44:

Fix up bungles of previous commits.

# This is the commit message Coding-Bunker#45:

Add preliminary support for attachments and roles.

# This is the commit message Coding-Bunker#46:

Add preliminary support for attachments and roles.

# This is the commit message Coding-Bunker#47:

Fix build errors, and rename role files to proper case.

# This is the commit message Coding-Bunker#48:

Clean code

Signed-off-by: Federico Guerinoni <[email protected]>

# This is the commit message Coding-Bunker#49:

Add latest version of API

Signed-off-by: Federico Guerinoni <[email protected]>

# This is the commit message Coding-Bunker#50:

Fix rebase error.

# This is the commit message Coding-Bunker#51:

Fix link to homepage in about dialog.

# This is the commit message Coding-Bunker#52:

Run js-beautify

# This is the commit message Coding-Bunker#53:

Mention temporary lack of support for voice channels.
# This is the commit message Coding-Bunker#54:

Omit version number, add features for channel and message objects.

# This is the commit message Coding-Bunker#55:

Clang-format misformatted files due to Qt Creator shenaniganry.

# This is the commit message Coding-Bunker#56:

Add preliminary support for attachments and roles.

# This is the commit message Coding-Bunker#57:

Add preliminary support for attachments and roles.

# This is the commit message Coding-Bunker#58:

Replace .value call with subscript operator for settings.

# This is the commit message Coding-Bunker#59:

Remove Git diff crud left by a previous commit.

# This is the commit message Coding-Bunker#60:

Fix formatting issue introduced by Qt Creator.

# This is the commit message Coding-Bunker#61:

Run clang-format

# This is the commit message Coding-Bunker#62:

Fix up bungles of previous commits.

# This is the commit message Coding-Bunker#63:

Add preliminary support for attachments and roles.

# This is the commit message Coding-Bunker#64:

Add preliminary support for attachments and roles.
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.

2 participants