Re-harmonize argument parsing between distutils and optparse CLIs #389

akx · 2016-04-14T08:25:28Z

I so hope this will be the last we see of this series of bugs...

Fixes #388
Fixes #390
Refs #384
Refs #311

codecov-io · 2016-04-14T08:51:05Z

Current coverage is `90.10%`

Merging #389 into master will increase coverage by +0.02% as of aecf236

@@            master   #389   diff @@
=====================================
  Files           24     24       
  Stmts         3935   3950    +15
  Branches         0      0       
  Methods          0      0       
=====================================
+ Hit           3545   3559    +14
  Partial          0      0       
- Missed         390    391     +1

Review entire Coverage Diff as of aecf236

Powered by Codecov. Updated on successful CI builds.

akx · 2016-04-14T19:00:59Z

Assigning @jtwang just to have some eyes on this.

ENuge · 2016-04-14T20:23:28Z

(Pretend I'm jtwang), shipit! I'm not super familiar with the different ways users can do option parsing, but the logic looks fine and you've got it tested. Hopefully this doesn't introduce any more regressions. :)

akx · 2016-04-15T05:21:42Z

Well, now that #390 appeared, I'm going to add a take a super duper close look at every single parameter that may have been mutated in #311. So re-review after that's done . . .

akx · 2016-04-17T15:59:03Z

Ping @ENuge / @jtwang again.

jtwang · 2016-04-17T17:07:56Z

babel/messages/frontend.py

@@ -39,6 +39,48 @@
    from configparser import RawConfigParser


+def listify_value(arg, split=None):


Will a single arg ever contain quotes/whitespace? Eg "foo "i am a.zip" bar"

That sort of usage will never have been (how's that for a temporal form?) supported by Babel... At least not the distutils frontend. (Sneaky edit: distutils, not optparse)

Double edit: Keyword strings should never contain significant whitespace, so that shouldn't matter, actually. Directory/path args are separated by other characters.

akx · 2016-04-19T16:08:31Z

@ENuge: Mind taking one more look?

ENuge · 2016-04-19T18:24:43Z

babel/messages/frontend.py

@@ -825,6 +882,7 @@ def _configure_command(self, cmdname, argv):
            strs = ["--%s" % name]
            if short:
                strs.append("-%s" % short)
+            strs.extend(cmdclass.option_aliases.get(name, ()))


Checking if this, combined with strs = ["--%s" % name], doesn't cause any problems? If name is "output" you'll end up with strs being ['--output', --output']. I don't have a super solid understanding of all the different options uh, options, so for all I know this intentional. If not an issue, shipit! Everything else looks good to me.

Oop, good catch.

The alias mapping should be 'output-file': ('--output',)...

(and as of ee8abd6, it is.)

Err, oh. I don't know why I was looking at the old version. Shipit!

… CLI Fixes python-babel#388 Refs python-babel#384 Refs python-babel#311

Now that arguments are more likely to be unicode than bytestrings, %r is not a good idea between Python versions.

…e mode

…comments`) Fixes python-babel#390

…python-babel#311 Vaguely refs python-babel#390

akx · 2016-04-19T19:05:12Z

So yeah, with that last comment fixed, merging. Will roll another bugfix release tomorrow.

akx added the bug label Apr 14, 2016

gitmate-bot added the pending review label Apr 14, 2016

akx mentioned this pull request Apr 14, 2016

2.3.3 distutils extract_messages misparses --keywords #388

Closed

akx assigned jtwang Apr 14, 2016

akx unassigned jtwang Apr 15, 2016

gitmate-bot added process/pending review size/L labels Apr 17, 2016

akx changed the title ~~Harmonize extraction keyword parsing between distutils and standalone CLI~~ Re-harmonize argument parsing between distutils and optparse CLIs Apr 17, 2016

akx mentioned this pull request Apr 17, 2016

pybabel extract forgot the -s CLI flag in version 2.3 #390

Closed

akx force-pushed the fix-388 branch from f070e9f to 97c1d18 Compare April 17, 2016 15:48

jtwang reviewed Apr 17, 2016
View reviewed changes

ENuge reviewed Apr 19, 2016
View reviewed changes

akx added 5 commits April 19, 2016 21:29

Harmonize extraction keyword parsing between distutils and standalone…

414aec5

… CLI Fixes python-babel#388 Refs python-babel#384 Refs python-babel#311

frontend: don't use unicode-variant %r for logging

72e97c1

Now that arguments are more likely to be unicode than bytestrings, %r is not a good idea between Python versions.

extract: don't die badly when no input paths are specified in optpars…

ad2057a

…e mode

Remind the optparse CLI about extract -s (a shorthand for `--strip-…

bb06614

…comments`) Fixes python-babel#390

Teach the optparse CLI about the parameter aliases it had forgotten in …

ee8abd6

…python-babel#311 Vaguely refs python-babel#390

akx force-pushed the fix-388 branch from 97c1d18 to ee8abd6 Compare April 19, 2016 18:29

akx merged commit 0e71d20 into python-babel:master Apr 19, 2016

akx deleted the fix-388 branch April 19, 2016 19:05

akx mentioned this pull request Apr 22, 2016

2.3.4 #394

Merged

pyup-bot mentioned this pull request Apr 11, 2017

Initial Update do3cc/blog#3

Closed

pyup-bot mentioned this pull request Apr 27, 2017

Pin babel to latest version 2.4.0 osteele/assignment-dashboard#24

Open

pyup-bot mentioned this pull request May 12, 2017

Initial Update tantale/intranet#43

Closed

pyup-bot mentioned this pull request Jul 4, 2017

Initial Update pelson/pyggybank#2

Merged

This was referenced Aug 17, 2017

Pin babel to latest version 2.4.0 springermac/pyinstaller#67

Merged

Pin babel to latest version 2.5.0 osteele/assignment-dashboard#51

Open

This was referenced Aug 26, 2017

Pin babel to latest version 2.5.0 nicfit/nicfit.py#129

Merged

Initial Update oddbird/oddsite#127

Closed

This was referenced Sep 4, 2017

Scheduled weekly dependency update for week 36 oddbird/oddsite#128

Merged

Initial Update rooty/flask-test#2

Open

pyup-bot mentioned this pull request Sep 14, 2017

Pin babel to latest version 2.5.1 osteele/assignment-dashboard#56

Open

pyup-bot mentioned this pull request Oct 17, 2017

Initial Update tracim/tracim#373

Closed

pyup-bot mentioned this pull request Nov 3, 2017

Update babel to 2.5.1 tracim/tracim#424

Closed

This was referenced Jan 15, 2018

Pin babel to latest version 2.5.2 osteele/assignment-dashboard#76

Open

Pin babel to latest version 2.5.3 osteele/assignment-dashboard#78

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-harmonize argument parsing between distutils and optparse CLIs #389

Re-harmonize argument parsing between distutils and optparse CLIs #389

akx commented Apr 14, 2016 •

edited

codecov-io commented Apr 14, 2016

akx commented Apr 14, 2016

ENuge commented Apr 14, 2016

akx commented Apr 15, 2016

akx commented Apr 17, 2016

jtwang Apr 17, 2016

akx Apr 17, 2016 •

edited

akx commented Apr 19, 2016

ENuge Apr 19, 2016

akx Apr 19, 2016 •

edited

akx Apr 19, 2016

ENuge Apr 19, 2016

akx commented Apr 19, 2016

		@@ -39,6 +39,48 @@
		from configparser import RawConfigParser


		def listify_value(arg, split=None):

Re-harmonize argument parsing between distutils and optparse CLIs #389

Re-harmonize argument parsing between distutils and optparse CLIs #389

Conversation

akx commented Apr 14, 2016 • edited

codecov-io commented Apr 14, 2016

Current coverage is 90.10%

akx commented Apr 14, 2016

ENuge commented Apr 14, 2016

akx commented Apr 15, 2016

akx commented Apr 17, 2016

jtwang Apr 17, 2016

Choose a reason for hiding this comment

akx Apr 17, 2016 • edited

Choose a reason for hiding this comment

akx commented Apr 19, 2016

ENuge Apr 19, 2016

Choose a reason for hiding this comment

akx Apr 19, 2016 • edited

Choose a reason for hiding this comment

akx Apr 19, 2016

Choose a reason for hiding this comment

ENuge Apr 19, 2016

Choose a reason for hiding this comment

akx commented Apr 19, 2016

akx commented Apr 14, 2016 •

edited

Current coverage is `90.10%`

akx Apr 17, 2016 •

edited

akx Apr 19, 2016 •

edited