Skip to content

Update regex usage for python 3.7#2225

Merged
gazpachoking merged 11 commits intodevelopfrom
regex_37
Jan 6, 2019
Merged

Update regex usage for python 3.7#2225
gazpachoking merged 11 commits intodevelopfrom
regex_37

Conversation

@gazpachoking
Copy link
Copy Markdown
Member

@gazpachoking gazpachoking commented Oct 12, 2018 •
edited
Loading

Motivation for changes:

Fix our usage of re.sub to work on python 3.7

Detailed changes:

  • Escapes any needed escapes in re.sub replacement patterns
  • Also includes circleci test enabling from Python3.7 support #2193

Addressed issues:

To Do:

  • Add UpgradeActions, any users specifying regex replacements in manipulate plugin which contain unknown escape patterns will have to double escape them. (I think it's unlikely many users are doing this though.)
  • Figure out proper fix for rtorrent client. Currently it works inconsistently across python versions and contains a hack to get test to pass. This has been delayed in rtorrent plugin escaping varies by python version #2297

@JohnDoee
Copy link
Copy Markdown

JohnDoee commented Oct 16, 2018

Can you elaborate on the rtorrent problems ? I'm unsure why Flexget escapes at all.

@gazpachoking
Copy link
Copy Markdown
Member Author

gazpachoking commented Oct 16, 2018

@JohnDoee The rtorrent issues are here:

Flexget/flexget/plugins/clients/rtorrent.py

Lines 314 to 316 in 8709b7d

# Values must be escaped if within params
# TODO: What are the escaping requirements? re.escape works differently on python 3.7+
params.append('d.%s.set=%s' % (key, re.escape(native_str(val))))

It's using re.escape to escape, which changed functionality on python 3.7+. No idea what really needs to be escaped there, so I didn't know how to fix it.

@JohnDoee
Copy link
Copy Markdown

JohnDoee commented Oct 16, 2018

So I looked through a bit of the rtorrent command argument parsing code.

My C++ isn't the best so it might be a bit wrong, anyways:

Argument parsing starts after first = found
https://github.com/rakshasa/rtorrent/blob/f0207ce6548026853ec57ab26f2e57a872f223bb/src/rpc/parse_commands.cc#L148

Variable parsing: https://github.com/rakshasa/rtorrent/blob/f0207ce6548026853ec57ab26f2e57a872f223bb/src/rpc/parse.cc#L175

Some helper commands: https://github.com/rakshasa/rtorrent/blob/f0207ce6548026853ec57ab26f2e57a872f223bb/src/rpc/parse.h#L53

Special characters in the given parsing that should probably be escaped for these use-cases:

  • , List separator
  • " Quote / Grouping an item
  • \ Escape characters
  • {} For a list ?
  • () Some grouping ? can be nested ?

Alternatively, throw quotes around, escape quotes and strip \

@chestm007
Copy link
Copy Markdown
Contributor

chestm007 commented Dec 8, 2018

I have the diff of this PR currently being applied to my AUR build flexget-git and its working fine for me. Perhaps we could get this PR in and make a sub/another task to fix rtorrent?

This was referenced Dec 8, 2018
Merged
Merged
@paranoidi
Copy link
Copy Markdown
Member

paranoidi commented Jan 4, 2019

I got some complaints with Python 3.6, committed fixes for those.

@paranoidi paranoidi self-requested a review January 4, 2019 00:07
@gazpachoking gazpachoking merged commit aa014b8 into develop Jan 6, 2019
This was referenced Jan 6, 2019
Closed
Closed
@gazpachoking gazpachoking deleted the regex_37 branch January 28, 2019 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@paranoidi paranoidi paranoidi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Flexget is incompatible with python 3.7

4 participants

@gazpachoking @JohnDoee @chestm007 @paranoidi