Skip to content

Conversation

@nephros
Copy link
Contributor

@nephros nephros commented Jan 15, 2022

corrects regular expressions:

  1. '+' has special meaning, escape it
  2. handle patches with "git-style" prefixes in the diff header, see Test Cases #232 (comment)

@Olf0
Copy link
Contributor

Olf0 commented Jan 16, 2022

Notes:

  1. Ad "1. '+' has special meaning, escape it":
    AFAIK only for PCREs (Pearl complex REs) and maybe for the obsolete EREs, but sed is special so often and I regularly become confused between the various RE-styles: It can do no harm to escape each +.
  2. Ad "2. handle patches with "git-style" prefixes in the diff header":
    AFAIU from the diff documentation, these are also simply path elements, but of relative paths. See GNU Diffutils: Detailed Description of Unified Format and especially GNU Diffutils: Avoiding Common Mistakes.
    I intended to argue that the character set for the first path element should be much more limited than [^/] (e.g., to [[:alnum:]]), but then realised that it could even contain backslash-quoted spaces ("\ "). So I have no better idea how to handle this.

@Olf0 Olf0 merged commit 7108079 into Olf0-patch-3 Jan 16, 2022
@Olf0 Olf0 deleted the Olf0-patch-3.1 branch January 16, 2022 14:17
Olf0 added a commit that referenced this pull request Jan 16, 2022
[pm_apply] Do convert multiple libpath references

[pm_apply]Simplify code

[pm_apply] Let log output be more expressive and consistent

[pm_apply] Enhance checks&balances and debug output

pm_apply: improve regexp (#237)

[pm_apply] (Re-)move misplaced log output and counter initialisation (#239)

[pm_apply] De-indent the whole function mangle_libpath
Copy link
Contributor

@Olf0 Olf0 left a comment

Choose a reason for hiding this comment

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

Looking O.K.

@nephros
Copy link
Contributor Author

nephros commented Jan 17, 2022

I intended to argue that the character set for the first path element should be much more limited than [^/] (e.g., to [[:alnum:]]), but then realised that it could even contain backslash-quoted spaces ("\ "). So I have no better idea how to handle this.

Yeah I was reluctant to use such broad matching. Correct one would be "any character allowed in a linux file path, including illegal characters if escaped properly". But even if such a regex term exists it would probably just look like line noise, hence not very maintainable.
So I opted for the easily readable might-break-in-some-corner-cases option.

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