This allows us to load workspace by using &Result<T, CommandError>. Even
though load_workspace() wouldn't be called more than once, consuming self
there is annoying.
I don't see a good reason to let e.g. "added diff" to match added text
inside a diff when we already allow "diff added" for that. Allowing
both means that we have to decide which should take precedence. With
the recent change to add labels for methods, we no longer depend on it
for the "timestamp author" case ("author timestamp" now
matches). Thanks to @yuja for noticing that dependency.
We now already have the "timestamp" label from the auto-labeling of
method calls.
Without the use of the "email" label in the previous commit, we would
now emit an unnecessary color code when the "author" label is added
but before the "timestamp" is added. So that patch wasn't just about
simplifiying the config :)
I'm going to make `parse_method_chain` also return a list of labels to
add, so we can make e.g. `author.timestamp()` automatically get
labeled with both "author" and "timestamp".
It's clearly the parser's job to split labels in a string provided by
the user. This patch moves the splitting we were doing in
`LabelTemplate` and `DynamicLabelTemplate` to the parser. In the
former case, the string isn't even provided by the user and it doesn't
contain whitespace, we can drop the splitting altogether.
An important part of the `jj log` output is the commit template. I
suspect we'll have many more tests for that, so let's move it to a
separate file. The main `test_log_command.rs` can focus on which
commits to display.
The `:` was a bit noisy. With upcoming color support, it seems unnecessary.
This should also be a little better for any tools that want to parse the
output.
Finally, `insta` also seems to want to rewrite test snapshots to be shorter.
They seem equivalent.
This will match `jj`'s behavior of being unable to resolve such conflicts or to
show readable diffs for them, in the pre-#978 state.
This is a fixup of 621293d7c6.
Since per-repo config may contain CLI settings, it must be visible to CLI.
Therefore, UserSettings::with_repo() -> RepoSettings isn't used, and its
implementation is nullified by this commit.
#616
It's unclear whether parse_args() or its caller should update LayeredConfigs.
--config-toml is processed by callee to apply --color early. -R/--repository
will be processed by caller since it will instantiate WorkspaceLoader.
Maybe --config-toml can be removed from EarlyArgs, and handle_early_args()
just updates ui state based on --color argument?
Thinking of config precedence, we'll probably need to store "base" (default +
env_base + user) config and "override" (env_overrides + --config-toml) config
separately to support repo config. Suppose env_overrides() is a temporary
value, it's better to override any file-derived values with $JJ_ env.
UserSettings will be instantiated after both user and repo configs are
loaded. We might want to add a wrapper for CLI settings, but I have no idea
how that should be structured. Let's use bare config::Config until then.
Still UserSettings is cloned to WorkspaceCommandHelper, but I think this is
slightly better since CommandHelper and WorkspaceCommandHelper are scoped
based on call stack. Perhaps, UserSettings can be shared by Arc or immutable
reference.
This prepares for migration from ui.settings() to command.settings(). We could
instead add workspace_command.settings(), but I'm not sure which would be less
bad. Anyway, these functions live in command layer, so taking CommandHelper
makes sense.
This is a temporary workaround. I think UserSettings can be constructed after
user, repo, and --config-toml values are all set. parse_args() will probably
return config::Config or its wrapper instead of mutating UserSettings.
I'm going to remove owned UserSettings from Ui so that UserSettings can be
instantiated after both user and repo configs are loaded. ui.cwd() belongs
to the same category (random environment stuff), and Ui doesn't depend on it,
so let's remove it first from Ui.
I'm not pretty sure if CommandHelper and WorkspaceCommandHelper should be
a permanent home for cwd and settings, but it works for now as CommandHelper
is immutable.
editor_name_from_settings() needs &mut Ui to show hint, but we're lucky that
the caller has a clone of UserSettings. This is one reason I want to remove
ui.settings(). A function taking (&mut Ui, &UserSettings) sounds reasonable,
but we can't pass in (&mut ui, ui.settings()) to it.
This and the subsequent patches prepare for the removal of ui.settings().
Ui will be a consumer of UserSettings (or config::Config) to make it clear
that Ui can be constructed before UserSettings is fully set up.
