Submitter | Sean Farley |
---|---|
Date | April 7, 2014, 9:17 p.m. |
Message ID | <0baffb232925978f2e7a.1396905421@laptop.local> |
Download | mbox | patch |
Permalink | /patch/4253/ |
State | Superseded |
Headers | show |
Comments
On 4/7/14, 2:17 PM, Sean Farley wrote: > # HG changeset patch > # User Sean Farley <sean.michael.farley@gmail.com> > # Date 1396902826 18000 > # Mon Apr 07 15:33:46 2014 -0500 > # Node ID 0baffb232925978f2e7a70f6cb2ebd770cda17d0 > # Parent 20427149f64472231b279bc17ade980cae402d8c > color: use valid_effect in configstyles > > diff --git a/hgext/color.py b/hgext/color.py > --- a/hgext/color.py > +++ b/hgext/color.py > @@ -326,13 +326,11 @@ def configstyles(ui): > continue > cfgeffects = ui.configlist('color', status) > if cfgeffects: > good = [] > for e in cfgeffects: > - if not _terminfo_params and e in _effects: > - good.append(e) > - elif e in _terminfo_params or e[:-11] in _terminfo_params: > + if valideffect(e): > good.append(e) > else: > ui.warn(_("ignoring unknown color/effect %r " > "(configured in color.%s)\n") > % (e, status)) > Why do this? The commit message doesn't explain why this is desirable, and the later patches don't seem to actually use it. I'd also do patches #1 and #2 as a single patch, since the changes together are a nice logical unit of change.
Durham Goode <durham@fb.com> writes: > On 4/7/14, 2:17 PM, Sean Farley wrote: >> # HG changeset patch >> # User Sean Farley <sean.michael.farley@gmail.com> >> # Date 1396902826 18000 >> # Mon Apr 07 15:33:46 2014 -0500 >> # Node ID 0baffb232925978f2e7a70f6cb2ebd770cda17d0 >> # Parent 20427149f64472231b279bc17ade980cae402d8c >> color: use valid_effect in configstyles >> >> diff --git a/hgext/color.py b/hgext/color.py >> --- a/hgext/color.py >> +++ b/hgext/color.py >> @@ -326,13 +326,11 @@ def configstyles(ui): >> continue >> cfgeffects = ui.configlist('color', status) >> if cfgeffects: >> good = [] >> for e in cfgeffects: >> - if not _terminfo_params and e in _effects: >> - good.append(e) >> - elif e in _terminfo_params or e[:-11] in _terminfo_params: >> + if valideffect(e): >> good.append(e) >> else: >> ui.warn(_("ignoring unknown color/effect %r " >> "(configured in color.%s)\n") >> % (e, status)) >> > Why do this? The commit message doesn't explain why this is desirable, > and the later patches don't seem to actually use it. I'd also do > patches #1 and #2 as a single patch, since the changes together are a > nice logical unit of change. Sure, I could fold these patches together if that's what people want. The reason this patch exists is to reduce code duplication and have one place were 'is this a valid effect name' is defined. I'll fold and update the patch description unless there are any objections.
Patch
diff --git a/hgext/color.py b/hgext/color.py --- a/hgext/color.py +++ b/hgext/color.py @@ -326,13 +326,11 @@ def configstyles(ui): continue cfgeffects = ui.configlist('color', status) if cfgeffects: good = [] for e in cfgeffects: - if not _terminfo_params and e in _effects: - good.append(e) - elif e in _terminfo_params or e[:-11] in _terminfo_params: + if valideffect(e): good.append(e) else: ui.warn(_("ignoring unknown color/effect %r " "(configured in color.%s)\n") % (e, status))