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

auth: fixup prompt and fail substitution #641

Merged
merged 1 commit into from
Jan 12, 2025
Merged

Conversation

PaideiaDilemma
Copy link
Collaborator

@PaideiaDilemma PaideiaDilemma commented Jan 6, 2025

This PR brings back fingerprint errors in $FAIL. That was regressed by #603.

BREAKING:

  • Removed $PROMPT variable. Either use $PAMPROMPT or $FPRINTPROMPT.
  • Removed $FPRINTMESSAGE. Use $FPRINTPROMPT instead. There is also
    $FPRINTFAIL.

@PaideiaDilemma PaideiaDilemma force-pushed the main branch 5 times, most recently from a44215b to 556cc86 Compare January 7, 2025 16:53
@PaideiaDilemma PaideiaDilemma changed the title auth: fixup prompt and fail placeholder substitution auth: fixup prompt and fail substitution Jan 7, 2025
@PaideiaDilemma
Copy link
Collaborator Author

After pondering with @alba4k we thought removing PROMPT was appropriate, since it is not that useful and we don't need to solve the "when to show which auth prompt" problem.

$FAIL and g_pAuth->getLastFailText() are now handled by the auth method directly (they specify the failText via g_pAuth->enqeueFail.

BREAKING:
- Removed $PROMPT variable. Either use $PAMPROMPT or $FPRINTPROMPT.
- Removed $FPRINTMESSAGE. Use $FPRINTPROMPT instead. There is also
  $FPRINTFAIL.
@PaideiaDilemma
Copy link
Collaborator Author

@vaxerski this one is ready.

Regarding the breaking variable changes (PROMPT & FPRINTMESSAGE), i could instead:

  • not remove them yet and say they will be removed in a future release in the wiki
  • replace them with a message to the user. smth like $PROMPT is Depreciated!

Let me know if you think any of those make more sense.
Personally i think not many people use those vars, so i think it is not that big of a problem.

@alba4k
Copy link
Contributor

alba4k commented Jan 10, 2025

I think they can be safely be removed, but if you really want to be safe the easiest solution is probably to just keep them working for a while and print a warning to stderr saying that the variable is deprecated and will be removed soon.

@vaxerski
Copy link
Member

why do the breaking changes? is there any good reason?

@alba4k
Copy link
Contributor

alba4k commented Jan 11, 2025

FPRINTMESSAGE is a misleading name, FPRINTFAIL is more consistent with the other variables and explains the purpose of the variable better

a unified PROMT variable doesn't really make sense with more than one auth method. Separating the variables makes more sense and gives the user more control

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

thanks. Wiki mr?

@vaxerski
Copy link
Member

oops, already there.

@vaxerski vaxerski merged commit 4f96437 into hyprwm:main Jan 12, 2025
1 check passed
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.

3 participants