( )⚙ D6720 config: fix fm.data() handling of defaultvalue

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6720

config: fix fm.data() handling of defaultvalue
ClosedPublic

Authored by navaneeth.suresh on Aug 7 2019, 9:50 AM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG60789444acd6: config: fix fm.data() handling of defaultvalue
Summary

This is a follow-up patch to rHG51a2e3102db2. This moves
fm.data() out of the if block in commands.config().

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit added a subscriber: pulkit.Aug 7 2019, 10:24 AM

This patch also needs a better description, something like: config: fix fm.data() handling of defaultvalue.

mercurial/commands.py
1884

A better fix will be to move fm.data(name=entryname) inside the if, i.e. where it was before https://phab.mercurial-scm.org/D6704.

And add fm.data(defaultvalue=...) below the if-else which will also move the defaultvalue to last in json output.

navaneeth.suresh retitled this revision from config: fix defaultvalue template keyword (patch 2 of 2) to config: fix fm.data() handling of defaultvalue.Aug 7 2019, 10:39 AM
navaneeth.suresh added inline comments.Aug 7 2019, 10:41 AM
mercurial/commands.py
1884

I did this but, couldn't see a difference in the test output.

pulkit added inline comments.Aug 7 2019, 11:07 AM
mercurial/commands.py
1884

ah, I guess the output is sorted. Anyway, the suggested one is a correct way to do this, because otherwise we are writing name to fm two times in case of uniquesel.

pulkit accepted this revision.Aug 7 2019, 5:09 PM
This revision is now accepted and ready to land.Aug 7 2019, 5:09 PM
navaneeth.suresh added a commit: rHG60789444acd6: config: fix fm.data() handling of defaultvalue.Aug 7 2019, 5:31 PM
Closed by commit rHG60789444acd6: config: fix fm.data() handling of defaultvalue (authored by navaneeth.suresh). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/commands.py (3 lines)
Mtests/test-config.t (4 lines)

Diff 16152

mercurial/commands.py

if fm.isplain(): if fm.isplain():
source = source or 'none' source = source or 'none'
value = value.replace('\n', '\\n') value = value.replace('\n', '\\n')
entryname = section + '.' + name entryname = section + '.' + name
if values and not (section in selsections or entryname in selentries): if values and not (section in selsections or entryname in selentries):
continue continue
fm.startitem() fm.startitem()
fm.condwrite(ui.debugflag, 'source', '%s: ', source) fm.condwrite(ui.debugflag, 'source', '%s: ', source)
if uniquesel: if uniquesel:
pulkitUnsubmitted
Not Done

A better fix will be to move fm.data(name=entryname) inside the if, i.e. where it was before https://phab.mercurial-scm.org/D6704.

And add fm.data(defaultvalue=...) below the if-else which will also move the defaultvalue to last in json output.

pulkit: A better fix will be to move `fm.data(name=entryname)` inside the if, i.e. where it was before…
navaneeth.sureshAuthorUnsubmitted
Done

I did this but, couldn't see a difference in the test output.

navaneeth.suresh: I did this but, couldn't see a difference in the test output.
pulkitUnsubmitted
Not Done

ah, I guess the output is sorted. Anyway, the suggested one is a correct way to do this, because otherwise we are writing name to fm two times in case of uniquesel.

pulkit: ah, I guess the output is sorted. Anyway, the suggested one is a correct way to do this…
fm.data(name=entryname, defaultvalue=defaultvalue) fm.data(name=entryname)
fm.write('value', '%s\n', value) fm.write('value', '%s\n', value)
else: else:
fm.write('name value', '%s=%s\n', entryname, value) fm.write('name value', '%s=%s\n', entryname, value)
fm.data(defaultvalue=defaultvalue)
matched = True matched = True
fm.end() fm.end()
if matched: if matched:
return 0 return 0
return 1 return 1
@command('continue', @command('continue',
dryrunopts, helpcategory=command.CATEGORY_CHANGE_MANAGEMENT, dryrunopts, helpcategory=command.CATEGORY_CHANGE_MANAGEMENT,

tests/test-config.t

$ hg showconfig Section $ hg showconfig Section
Section.KeY=Case Sensitive Section.KeY=Case Sensitive
Section.key=lower case Section.key=lower case
$ hg showconfig Section -Tjson $ hg showconfig Section -Tjson
[ [
{ {
"defaultvalue": null,
"name": "Section.KeY", "name": "Section.KeY",
"source": "*.hgrc:*", (glob) "source": "*.hgrc:*", (glob)
"value": "Case Sensitive" "value": "Case Sensitive"
}, },
{ {
"defaultvalue": null,
"name": "Section.key", "name": "Section.key",
"source": "*.hgrc:*", (glob) "source": "*.hgrc:*", (glob)
"value": "lower case" "value": "lower case"
} }
] ]
$ hg showconfig Section.KeY -Tjson $ hg showconfig Section.KeY -Tjson
[ [
{ {
"defaultvalue": null, "defaultvalue": null,
"name": "Section.KeY", "name": "Section.KeY",
"source": "*.hgrc:*", (glob) "source": "*.hgrc:*", (glob)
"value": "Case Sensitive" "value": "Case Sensitive"
} }
] ]
$ hg showconfig -Tjson | tail -7 $ hg showconfig -Tjson | tail -7
},
{ {
"defaultvalue": null,
"name": "*", (glob) "name": "*", (glob)
"source": "*", (glob) "source": "*", (glob)
"value": "*" (glob) "value": "*" (glob)
} }
] ]
Test empty config source: Test empty config source: