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

Strip quote characters in macro expansion if we do not split the result #2788

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

mlschroe
Copy link
Collaborator

A problem with the old handling of %quote was that it leaked to the outside. This commit strips the quote characters if they are not used in argument splitting.

This makes the %shescape macro work use all arguments like we
already do in the %quote macro.

I.e.:
    %{shescpae:hello world} -> 'hello world'
    %{shescpae hello world} -> 'hello' 'world'
A problem with the old handling of %quote was that it leaked to
the outside. This commit strips the quote characters if they are
not used in argument splitting.

We also keep track of the quotes when defining macros so that
we stay compatible to the old implementation.
@mlschroe
Copy link
Collaborator Author

This is actually a quite powerful. Try this:

%define hey() {
  this is hey
  %{shescape %**}
  %**
  ho
}

%global array foo bar %{quote:hello world} %%{quote:huhu foo}
%global xx x1 %{quote:x2 x3}
%global array %array %{quote:aa %%xx} %%xx
%global array %array %{quote:rpm is great} rpm is great
%{shescape %array}
%array

%{hey %array}

@mlschroe mlschroe changed the title Strip quote characters in macro expansion if we do not split the result RFC: Strip quote characters in macro expansion if we do not split the result Nov 29, 2023
@pmatilai
Copy link
Member

pmatilai commented Dec 8, 2023

Fun stuff 😁 Thanks for the patch!

@pmatilai pmatilai merged commit fff2111 into rpm-software-management:master Dec 8, 2023
1 check passed
@pmatilai pmatilai added the macros label Dec 8, 2023
@mlschroe
Copy link
Collaborator Author

mlschroe commented Dec 8, 2023

But... it wasn't meant to be merged yet, thus the "RFC" in the name. There's testcases missing, and I don't like that RPMEXPAND_HAVE_QUOTED is actually a result, not an input. Next time I'll add the "Dont't merge yet" label.

@pmatilai
Copy link
Member

Oh. I did actually notice the RFC, but it seemed like a reasonably complete patch so thought why not.

The best way to ensure PRs dont get accidentally merged is to create it as a draft PR, that way it's enforced at GH level.

@pmatilai
Copy link
Member

Do you want me to revert it for the time being?

@mlschroe
Copy link
Collaborator Author

No, the code works fine. I'll just to another pull request to tidy it up a bit and add some test cases.

@dmnks dmnks changed the title RFC: Strip quote characters in macro expansion if we do not split the result Strip quote characters in macro expansion if we do not split the result Mar 22, 2024
@dmnks dmnks added the bug label Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants