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

fix: over-permissive regex for added/deleted file meta header #421

Merged
merged 1 commit into from
Oct 22, 2021

Conversation

rwe
Copy link
Contributor

@rwe rwe commented Oct 21, 2021

Noticed this because the line itself was filtered out of diffs in this codebase.

The previous pattern uselessly included .*, unlike all the other patterns, and so would erroneously match on actual diff content lines.

This also tightens the pattern since the file mode will always be six octal (0-7) chars; and in the git codebase the format string is: "%s%snew file mode %06o%s\n".

@scottchiefbaker
Copy link
Contributor

This PR also looks good. I just noticed that last PR was against master.

Per our contribution guidelines, all PRs should be against next. If you redo this against next I'll merge it.

@rwe
Copy link
Contributor Author

rwe commented Oct 22, 2021

@scottchiefbaker Ah, I missed that, thanks! It looks like the next branch is not quite up-to-date with master currently: it includes #417 (npm use flatpack) and my previous PR (#422).

Would you prefer I create PRs for those two branches into next first to sync them up? Or would you prefer to just ff-merge master into next?

For this one, I think I can just rebase it back to the common base (i.e. not including the npm flatpack PR).

The previous pattern uselessly included `.*`, unlike all the other
patterns, and so would erroneously match on actual diff content lines.

Noticed this because the line itself was filtered out of diffs in this
codebase.

This also tightens the pattern since the file mode will always be six
octal (0-7) chars; and in the git codebase the format string is:
  "%s%snew file mode %06o%s\n"
@rwe rwe force-pushed the fix-file-meta-regex branch from a584a20 to fc8e7ee Compare October 22, 2021 18:59
@rwe rwe changed the base branch from master to next October 22, 2021 19:02
@rwe
Copy link
Contributor Author

rwe commented Oct 22, 2021

This PR now targets next

@scottchiefbaker scottchiefbaker merged commit b1deaee into so-fancy:next Oct 22, 2021
@scottchiefbaker
Copy link
Contributor

Excellent thanks for the simple concise diff. Makes my job a lot easier.

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