Skip to content

Fix check for OAuth::PromptValuesSupported - #1091

Open
TheOneRing wants to merge 2 commits into
mainfrom
work/enum2
Open

Fix check for OAuth::PromptValuesSupported#1091
TheOneRing wants to merge 2 commits into
mainfrom
work/enum2

Conversation

@TheOneRing

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Utility::enumValues() uses std::views but utility.h does not include <ranges>, which is likely to cause compilation failures in translation units that include this header.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates OAuth prompt-value handling to correctly treat OAuth::PromptValuesSupported as a Qt flags enum, and introduces a shared Utility::enumValues() helper (with a unit test) to iterate enum/flag values via Qt meta-object data.

Changes:

  • Register OAuth::PromptValuesSupported as a flags enum (Q_FLAG) and use meta-enum iteration to stringify supported prompt flags.
  • Add Utility::enumValues() for iterating QMetaEnum values, with a new unit test exercising a namespace-scoped flags enum.
  • Extend test utilities with a Q_NAMESPACE + Q_FLAG_NS enum for validating enum-utility behavior.
File summaries
File Description
test/testutils/testutils.h Adds a namespace-scoped flags enum for testing enum meta-utilities.
test/testutility.cpp Adds a unit test for Utility::enumValues().
src/libsync/creds/oauth.h Switches PromptValuesSupported registration from Q_ENUM to Q_FLAG.
src/libsync/creds/oauth.cpp Uses Utility::enumValues() to stringify all supported prompt flags.
src/libsync/common/utility.h Introduces Utility::enumValues() based on QMetaEnum.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/libsync/common/utility.h
Comment thread src/libsync/common/utility.h
Comment thread test/testutils/testutils.h Outdated

@guruz guruz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably good if it works, maybe @ogoffart has a better idea (without exotic new C++ features)

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants