This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7628

config: move reading of defaultrc/ files from ui to rcutil
AbandonedPublic

Authored by martinvonz on Dec 12 2019, 6:44 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

For PyOxidizer, we'll need to read the default configs using the
resources module. This prepares for putting that call in rcutil.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

martinvonz created this revision.Dec 12 2019, 6:44 PM
indygreg added a subscriber: indygreg.Dec 23 2019, 12:28 PM

The patch seems functionally correct. But I have a question about whether this is the desired approach. See inline comment.

mercurial/rcutil.py
102

Do you think it would be better to change the return type to expose a resource type, which the caller can resolve accordingly? Or maybe do away with leaving it up to the caller to open and read paths? It just feels weird for some I/O to be performed in this function while some I/O is deferred to the caller.

But I don't have this code paged in, so maybe my comment is off base.

martinvonz abandoned this revision.Jan 23 2020, 5:04 PM

This was probably also obsoleted by D7629

Revision Contents
Changeset List

PathPackages
Mmercurial/rcutil.py (13 lines)

Diff 18658

mercurial/rcutil.py

# rcutil.py - utilities about config paths, special config sections etc. # rcutil.py - utilities about config paths, special config sections etc.
# #
# Copyright Mercurial Contributors # Copyright Mercurial Contributors
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
import os import os
from . import ( from . import (
config,
encoding, encoding,
pycompat, pycompat,
util, util,
) )
from .utils import resourceutil from .utils import resourceutil
if pycompat.iswindows: if pycompat.iswindows:
# assume HGRCPATH is all about user configs so environments can be # assume HGRCPATH is all about user configs so environments can be
# overridden. # overridden.
_rccomponents = [envrc] _rccomponents = [envrc]
for p in encoding.environ[b'HGRCPATH'].split(pycompat.ospathsep): for p in encoding.environ[b'HGRCPATH'].split(pycompat.ospathsep):
if not p: if not p:
continue continue
_rccomponents.extend((b'path', p) for p in _expandrcpath(p)) _rccomponents.extend((b'path', p) for p in _expandrcpath(p))
else: else:
default_entries = []
for path in defaultrcpath():
cfg = config.config()
cfg.read(path)
for section in cfg:
for name, value in cfg.items(section):
source = cfg.source(section, name)
default_entries.append((section, name, value, source))
_rccomponents = [(b'items', default_entries)]
indygregUnsubmitted
Not Done

Do you think it would be better to change the return type to expose a resource type, which the caller can resolve accordingly? Or maybe do away with leaving it up to the caller to open and read paths? It just feels weird for some I/O to be performed in this function while some I/O is deferred to the caller.

But I don't have this code paged in, so maybe my comment is off base.

indygreg: Do you think it would be better to change the return type to expose a `resource` type, which…
normpaths = lambda paths: [ normpaths = lambda paths: [
(b'path', os.path.normpath(p)) for p in paths (b'path', os.path.normpath(p)) for p in paths
] ]
_rccomponents = normpaths(defaultrcpath() + systemrcpath()) _rccomponents.extend(normpaths(systemrcpath()))
_rccomponents.append(envrc) _rccomponents.append(envrc)
_rccomponents.extend(normpaths(userrcpath())) _rccomponents.extend(normpaths(userrcpath()))
return _rccomponents return _rccomponents
def defaultpagerenv(): def defaultpagerenv():
'''return a dict of default environment variables and their values, '''return a dict of default environment variables and their values,
intended to be set before starting a pager. intended to be set before starting a pager.
''' '''
return {b'LESS': b'FRX', b'LV': b'-c'} return {b'LESS': b'FRX', b'LV': b'-c'}