Skip to content

Conversation

@ayousuf23
Copy link
Contributor

@ayousuf23 ayousuf23 commented Jan 20, 2022

PR Summary

When using tab completion, functions with - are wrongfully selected. In order to prevent this, an additional check is added in this PR to prevent tab completion when the word to complete is a single dash.

PR Context

This PR resolves #16577. It simply adds an additional condition (making sure the word to complete is not a single dash) that should be satisfied before PowerShell performs tab completion.

PR Checklist

@ghost ghost assigned PaulHigin Jan 20, 2022
@PaulHigin PaulHigin requested a review from daxian-dbw January 20, 2022 22:05
@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Jan 21, 2022
@ayousuf23
Copy link
Contributor Author

@iSazonov I can't see your suggestion/code review anymore. Can you please add it again?

@ayousuf23
Copy link
Contributor Author

ayousuf23 commented Jan 26, 2022

@iSazonov Nevermind. I found your suggestion and updated the code.

@iSazonov
Copy link
Collaborator

Nevermind. I found your suggestion and updated the code.

@ayousuf23 Sorry, I removed the post soon because it was wrong. Please remove your last commit and add a test for scenario you fix.

@ayousuf23 ayousuf23 force-pushed the issue-16577 branch 2 times, most recently from c8875e5 to 122b4d9 Compare January 27, 2022 16:26
@ayousuf23
Copy link
Contributor Author

@iSazonov I reverted the most recent commit. Where should I add the test?

@iSazonov
Copy link
Collaborator

@iSazonov I reverted the most recent commit. Where should I add the test?

I think in TabCompletion.Tests.ps1

@ayousuf23
Copy link
Contributor Author

@iSazonov I added a test.

@iSazonov iSazonov closed this Jan 30, 2022
@iSazonov iSazonov reopened this Jan 30, 2022
@ayousuf23
Copy link
Contributor Author

@iSazonov Is there anything remaining for me to do in order to get the PR merged?

@iSazonov
Copy link
Collaborator

iSazonov commented Feb 1, 2022

@ayousuf23 We are waiting @daxian-dbw review.

@ayousuf23
Copy link
Contributor Author

@daxian-dbw Can you please review this PR?

@ghost ghost added the Review - Needed The PR is being reviewed label Feb 9, 2022
@ghost
Copy link

ghost commented Feb 9, 2022

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

@pull-request-quantifier-deprecated

This PR has 7 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Small
Size       : +6 -1
Percentile : 2.8%

Total files changed: 2

Change summary by file extension:
.cs : +1 -1
.ps1 : +5 -0

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detetcted.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

Copy link
Member

@daxian-dbw daxian-dbw left a comment

Choose a reason for hiding this comment

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

@ayousuf23 Sorry for the delay of review.
Thanks for the contribution! I made a minor update and added a comment.

@PaulHigin PaulHigin removed the Review - Needed The PR is being reviewed label Feb 9, 2022
@PaulHigin PaulHigin merged commit a9a235d into PowerShell:master Feb 9, 2022
@ghost
Copy link

ghost commented Feb 24, 2022

🎉v7.3.0-preview.2 has been released which incorporates this pull request.:tada:

Handy links:

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

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log Extra Small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

functions with - on the name wrongfully selected for tab completion

4 participants