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

feat: add topic separator option #111

Merged
merged 2 commits into from
Feb 27, 2021
Merged

feat: add topic separator option #111

merged 2 commits into from
Feb 27, 2021

Conversation

RasPhilCo
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Feb 23, 2021

Codecov Report

Merging #111 (68d9dd5) into master (5292ed1) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #111   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files           3       3           
  Lines          15      15           
  Branches        2       2           
======================================
  Misses         15      15           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5292ed1...68d9dd5. Read the comment docs.

@RasPhilCo RasPhilCo changed the title feat: add spaced commands syntax option feat: add topic separator option Feb 25, 2021
@RasPhilCo RasPhilCo requested review from amphro and chadian February 25, 2021 02:53
Copy link
Contributor

@amphro amphro left a comment

Choose a reason for hiding this comment

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

Nice! A few more tests around finding commands might be nice.

src/config/config.ts Show resolved Hide resolved
const findId = (id: string, next: string[]): string | undefined => {
const idPresnet = (id: string) => ids.includes(id)
if (idPresnet(id) && !idPresnet(`${id}:${next[0]}`)) return id
if (next.length === 0 || next[0] === '--') return
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this also return id?

Copy link
Contributor Author

@RasPhilCo RasPhilCo Feb 27, 2021

Choose a reason for hiding this comment

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

This is returning no match b/c the line would return the last valid id found. This line is saying, you didn't find anything to return (line above it) and you have no further argv to explore.

@RasPhilCo RasPhilCo merged commit b3ca07f into master Feb 27, 2021
@RasPhilCo RasPhilCo deleted the spaced-cmds branch February 27, 2021 00:17
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.

2 participants