Page MenuHomePhabricator

config: drop debug messages saying where config was read from
ClosedPublic

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

Details

Summary

hg config --debug includes lines like this:

set config by: $EDITOR

but also lines like this:

$EDITOR: ui.editor=emacs -nw

The set config by messages don't seem to provide much additional
information over what we get from the $EDITOR:-type message. I could
imagine wanting to see which values got overriden by a later entry,
but that information is already not present. So let's just remove the
first type of output. My next patch would otherwise amplify the
redundant output (there would be one set config by for each line in
mergetools.rc).

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

martinvonz created this revision.Dec 12 2019, 6:44 PM

On Windows, you can set a registry key to one or more *.ini files or directories that get read in, so it seems slightly handy to know the order in which files are processed. I know I've used that mechanism on various systems to find directories in which to stash things too.

That said, I don't think I feel strongly about keeping it if it makes the next patch messy. (I did a quick skim of it, but didn't see why that would be.)

On Windows, you can set a registry key to one or more *.ini files or directories that get read in, so it seems slightly handy to know the order in which files are processed. I know I've used that mechanism on various systems to find directories in which to stash things too.
That said, I don't think I feel strongly about keeping it if it makes the next patch messy. (I did a quick skim of it, but didn't see why that would be.)

It means that every line in merge-tools.rc would get two lines in this output (one line saying "set value from ..." or something like that).

On Windows, you can set a registry key to one or more *.ini files or directories that get read in, so it seems slightly handy to know the order in which files are processed. I know I've used that mechanism on various systems to find directories in which to stash things too.
That said, I don't think I feel strongly about keeping it if it makes the next patch messy. (I did a quick skim of it, but didn't see why that would be.)

It means that every line in merge-tools.rc would get two lines in this output (one line saying "set value from ..." or something like that).

I'm also happy to drop just the "set item" lines. Better?

On Windows, you can set a registry key to one or more *.ini files or directories that get read in, so it seems slightly handy to know the order in which files are processed. I know I've used that mechanism on various systems to find directories in which to stash things too.
That said, I don't think I feel strongly about keeping it if it makes the next patch messy. (I did a quick skim of it, but didn't see why that would be.)

It means that every line in merge-tools.rc would get two lines in this output (one line saying "set value from ..." or something like that).

I'm also happy to drop just the "set item" lines. Better?

I think so, thanks.

One of the last hunks dropped here mentions "set by $EDITOR", but then never mentions EDITOR again. That seems confusing if you don't know what that setting applies to and the precedence rules, and not real helpful if you do.

On Windows, you can set a registry key to one or more *.ini files or directories that get read in, so it seems slightly handy to know the order in which files are processed. I know I've used that mechanism on various systems to find directories in which to stash things too.
That said, I don't think I feel strongly about keeping it if it makes the next patch messy. (I did a quick skim of it, but didn't see why that would be.)

It means that every line in merge-tools.rc would get two lines in this output (one line saying "set value from ..." or something like that).

I'm also happy to drop just the "set item" lines. Better?

I think so, thanks.
One of the last hunks dropped here mentions "set by $EDITOR", but then never mentions EDITOR again. That seems confusing if you don't know what that setting applies to and the precedence rules, and not real helpful if you do.

Okay, will upload an update that adds back the read config from: lines a few minutes.

martinvonz edited the summary of this revision. (Show Details)Dec 16 2019, 6:34 PM
martinvonz updated this revision to Diff 18779.
pulkit added a subscriber: pulkit.Dec 17 2019, 6:45 AM

After recent iteration, commit message needs to be reworked. Maybe I am biased because I know what the old version was and is still parsing the commit message that way.

After recent iteration, commit message needs to be reworked. Maybe I am biased because I know what the old version was and is still parsing the commit message that way.

I did rework the message, but can see that mentioning the read config from: lines is now misleading. I'll remove that.

pulkit accepted this revision.Dec 17 2019, 11:06 AM
This revision is now accepted and ready to land.Dec 17 2019, 11:06 AM