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

Points: Also support prefix increments #441

Open
2 of 4 tasks
Asartea opened this issue Oct 1, 2023 · 3 comments
Open
2 of 4 tasks

Points: Also support prefix increments #441

Asartea opened this issue Oct 1, 2023 · 3 comments

Comments

@Asartea
Copy link
Contributor

Asartea commented Oct 1, 2023

Complete the following REQUIRED checkboxes:

  • I have thoroughly read and understand The Odin Project Contributing Guide

  • The title of this issue follows the command name: brief description of request format, e.g. /help: add optional @user parameter

The following checkbox is OPTIONAL:

  • I would like to be assigned this issue to work on it

1. Description of the Feature Request:

Sometimes people use the wrong form of increment (++prefix rather than postfix++), when trying to give someone points, which doesn't work. It would be neat if this also worked

2. Acceptance Criteria:

  • ++@example works the same as @example++

3. Additional Information:

@Asartea Asartea added the Status: Needs Review This issue/PR needs an initial or additional review label Oct 1, 2023
@01zulfi
Copy link
Member

01zulfi commented Feb 24, 2024

That sounds cool, I'm not sure how much more complicated it will make the points regex

`(?<!\\S)${userRegex}\\s?(${doublePointsPlusRegex}|${plusRegex}|${starRegex})(?!\\S)`,

@TheOdinProject/maintainers what do we think?

@01zulfi 01zulfi removed the Status: Needs Review This issue/PR needs an initial or additional review label Feb 24, 2024
@thatblindgeye
Copy link
Contributor

Instead of using each of the different points regex individually in the awardPoints.regex, we could create a new variable so that the "main" regex isn't as long:

const starRegex...
const plusRegex...
const doublePointsPlusRegex...

const givePointsRegex = `(${doublePointsPlusRegex}|${plusRegex}|${starRegex})`

Then just check whether for "userRegex followed by givePointsRegex | givePointsRegex followed by userRegex". Doesn't really make it less complicated, but maybe a little easier to read?

`(?<!\\S)(${userRegex}\\s?${givePointsRegex}|${givePointsRegex}\\s?${userRegex})(?!\\S)`

@MaoShizhong
Copy link
Contributor

The above sounds very reasonable. I like the idea of supporting pre-increment.

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

No branches or pull requests

4 participants