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

Calculator - Human multiplication expressions #24655

Merged
merged 3 commits into from
Mar 10, 2023

Conversation

jjavierdguezas
Copy link
Contributor

@jjavierdguezas jjavierdguezas commented Mar 8, 2023

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

I took into account the combination of numbers (num), constants (const), functions (func) and expressions in parentheses ((exp)). The blank spaces between them were also considered. The solution is based on regexes. Some combinations were not handled as they are not common (but some could be handled if necessary)

  • const const: pie = pi * e
  • const num: pi3 = pi * 3 not handled because it is not common
  • const func: eln(100) = e * ln(100)
  • const (exp): pi(1+1) = pi * (1+1)
  • num const: 2pi = 2 * pi
  • num num: not handled
  • num func: 2log10(100) = 2 * log10(100)
  • num (exp): 2(3+4) = 2 * (3+4)
  • func const: log10(100)pi = log10(100) * pi not handled because it is not common
  • func num: log10(100)3 = log10(100) * 3 not handled because it is not common
  • func func: sin(pi)cos(pi) = sin(pi) * cos(pi)
  • func (exp): log10(100)(2+3) = log10(100) * (2+3)
  • (exp) const: (1+2)pi = (1+2) * pi not handled because it is not common
  • (exp) num: (1+2)4 = (1+2) * 4 not handled because it is not common
  • (exp) func: (1+1)cos(pi) = (1+1) * cos(pi)
  • (exp) (exp): (1+1)(2+2) = (1+1) * (2+2)

Validation Steps Performed

Copy link
Collaborator

@htcfreek htcfreek left a comment

Choose a reason for hiding this comment

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

@jjavierdguezas
I took a first fast look at the code. Found a small typo in all test method names.

Please fill the PR checklist. And it would be great if you can create a PR for our docs page later.

[DataRow("pipipie", "pi * pi * pi * e")]
[DataRow("(1+1)(3+2)(1+1)(1+1)", "(1+1) * (3+2) * (1+1) * (1+1)")]
[DataRow("(1+1) (3+2) (1+1)(1+1)", "(1+1) * (3+2) * (1+1) * (1+1)")]
public void RightHumaMultiplicationExpressionTransformation(string typedString, string expectedQuery)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Huma -> Human. (Same for the other checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

solved

- fix some typos
- updated dev docs
- added PR examples to tests
- improve method naming style
@jjavierdguezas
Copy link
Contributor Author

@jjavierdguezas I took a first fast look at the code. Found a small typo in all test method names.

Please fill the PR checklist. And it would be great if you can create a PR for our docs page later.

Hi @htcfreek , if I didn't add new files, do I have to check the "New binaries" checkbox?

@htcfreek
Copy link
Collaborator

htcfreek commented Mar 8, 2023

@jjavierdguezas I took a first fast look at the code. Found a small typo in all test method names.

Please fill the PR checklist. And it would be great if you can create a PR for our docs page later.

Hi @htcfreek , if I didn't add new files, do I have to check the "New binaries" checkbox?

No need to check this box.

Copy link
Collaborator

@stefansjfw stefansjfw left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for your contribution!

doc/devdocs/modules/launcher/plugins/calculator.md Outdated Show resolved Hide resolved
Co-authored-by: Stefan Markovic <[email protected]>
@stefansjfw stefansjfw merged commit cc708e7 into microsoft:main Mar 10, 2023
@Jay-o-Way Jay-o-Way mentioned this pull request Mar 28, 2023
BLM16 pushed a commit to BLM16/PowerToys that referenced this pull request Jun 22, 2023
* fixes microsoft#20187

* handles PR reviews
- fix some typos
- updated dev docs
- added PR examples to tests
- improve method naming style

* Fix typo

Co-authored-by: Stefan Markovic <[email protected]>

---------

Co-authored-by: José Javier Rodríguez Zas <[email protected]>
Co-authored-by: Stefan Markovic <[email protected]>
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