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.

Sign up for GitHub

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

Jump to bottom

Duplicate targets should be differentiated by text and line number #666

Closed
somewhatabstract opened this issue Jun 15, 2021 · 0 comments · Fixed by #847
Closed

Duplicate targets should be differentiated by text and line number #666

somewhatabstract opened this issue Jun 15, 2021 · 0 comments · Fixed by #847
Assignees
@somewhatabstract
Labels
bug Something isn't working
Milestone
CheckSync 3.0.0

Comments

@somewhatabstract
Copy link
Owner

somewhatabstract commented Jun 15, 2021 •
edited
Loading

Duplicate targets are going to have the same text and therefore share the same auto-fixes which breaks the way our autofixer works. The autofixer should track line numbers and use that to differentiate the targets.

In other words, if a duplicate target has exactly the same text as its predecessor then they will share their fixes, which isn't really what we want since if the first fix is to delete as a duplicate, it might delete both! A TODO is in place to look at addressing this by using both text and line number to apply the correct fix.

See

checksync/src/fix-file.js

Lines 75 to 79 in d96418a

// If there are multiple lines with the exact same text,
// they will both receive the same fix till we track the line
// number.
// TODO: Track the line number so we can work with duplicate
// text.

@somewhatabstract somewhatabstract added the bug Something isn't working label Jun 15, 2021
@somewhatabstract somewhatabstract self-assigned this Jun 15, 2021
@somewhatabstract somewhatabstract mentioned this issue Jun 15, 2021
Merged
@somewhatabstract somewhatabstract mentioned this issue Nov 22, 2021
Merged
somewhatabstract added a commit that referenced this issue Dec 15, 2021
@somewhatabstract
Support checksyncrc files (#812)
1b7b343 
## Summary:
This adds support for configuration files so that we don't have to supply all options on the command line.
It also adds test coverage for our output sink class and some other light tweaks.

This is the last piece for CheckSync v3! (Though I may try addressing #666)

Note to reviewers:

Much of this diff is test and snapshots updates. I recommend filtering out `.snap` files and skipping over `_test.js` files to look at the JSON and JS file changes.

Fixes #664

## Test plan:
`yarn test`
`yarn flow`

Author: somewhatabstract

Reviewers: somewhatabstract, kevinbarabash, jeremywiebe

Required Reviewers:

Approved By: kevinbarabash

Checks: ⚪️ CodeQL, ✅ codecov/project, ✅ Test and build (macOS-latest, 16.x), ✅ Test and build (macOS-latest, 14.x), ✅ Test and build (macOS-latest, 12.x), ✅ Test and build (ubuntu-latest, 16.x), ✅ Test and build (ubuntu-latest, 14.x), ✅ Test and build (ubuntu-latest, 12.x), ✅ Test and build (windows-latest, 16.x), ✅ Test and build (windows-latest, 14.x), ✅ Test and build (windows-latest, 12.x), ✅ Integration tests (windows-latest, 16.x), ✅ Integration tests (windows-latest, 14.x), ✅ Integration tests (windows-latest, 12.x), ✅ Integration tests (macOS-latest, 16.x), ✅ Integration tests (macOS-latest, 14.x), ✅ Integration tests (macOS-latest, 12.x), ✅ Integration tests (ubuntu-latest, 16.x), ✅ Integration tests (ubuntu-latest, 14.x), ✅ Integration tests (ubuntu-latest, 12.x), ✅ Analyze (javascript), ✅ Lint and flow check (ubuntu-latest, 16.x), ✅ Update test coverage (ubuntu-latest, 16.x), ⏭  dependabot

Pull Request URL: #812
@somewhatabstract somewhatabstract added this to the CheckSync 3.0.0 milestone Dec 15, 2021
@somewhatabstract somewhatabstract mentioned this issue Dec 16, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@somewhatabstract somewhatabstract

Labels
bug Something isn't working
Projects
None yet
Milestone
CheckSync 3.0.0
Development

Successfully merging a pull request may close this issue.

Make sure fixes are applied to the correct lines somewhatabstract/checksync
1 participant
@somewhatabstract