Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add template aliases with node symbol configs #3501

Merged
merged 2 commits into from
Apr 15, 2024
Merged

Conversation

algmyr
Copy link
Contributor

@algmyr algmyr commented Apr 13, 2024

Possible future defaults, for opt-in testing.

Considering we'll keep these in the configs for now I'll use the split setup I use in my own config where labeling and symbols are separate, which grants you some goodies like the working change symbol changing color on a conflicted commit.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

@algmyr algmyr force-pushed the algmyr/push-tkssmzytmzrw branch from b6f062b to df8cacf Compare April 13, 2024 20:12
@algmyr algmyr mentioned this pull request Apr 13, 2024
4 tasks
Copy link
Contributor

@yuja yuja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

cli/src/config/templates.toml Outdated Show resolved Hide resolved
cli/src/config/templates.toml Outdated Show resolved Hide resolved
@algmyr algmyr force-pushed the algmyr/push-tkssmzytmzrw branch from df8cacf to 5ee0e77 Compare April 14, 2024 21:43
cli/src/config/templates.toml Outdated Show resolved Hide resolved
cli/src/config/templates.toml Outdated Show resolved Hide resolved
@algmyr algmyr force-pushed the algmyr/push-tkssmzytmzrw branch from 5ee0e77 to f79f5d0 Compare April 15, 2024 19:34
@algmyr algmyr force-pushed the algmyr/push-tkssmzytmzrw branch from f79f5d0 to 9188018 Compare April 15, 2024 19:53
@algmyr
Copy link
Contributor Author

algmyr commented Apr 15, 2024

Note: I'll send a patch to move the top-level label("node", ..) to caller just like label("op_log", ..).

Is the plan to make it node log ... and node op_log ... or similar (maybe flipped order)?

@algmyr algmyr merged commit af18572 into main Apr 15, 2024
16 checks passed
@algmyr algmyr deleted the algmyr/push-tkssmzytmzrw branch April 15, 2024 20:21
@yuja
Copy link
Contributor

yuja commented Apr 16, 2024

Note: I'll send a patch to move the top-level label("node", ..) to caller just like label("op_log", ..).

Is the plan to make it node log ... and node op_log ... or similar (maybe flipped order)?

I think one-word "log_node"/"op_log_node" labels are more consistent, but "log node"/"op_log node" are also good.

@algmyr
Copy link
Contributor Author

algmyr commented Apr 16, 2024

The advantage of two words is that if you don't need to distinguish between the two you can just specify node ....
(Also kinda makes me think I should have renamed the current op/commit label to just current in both cases)

@yuja
Copy link
Contributor

yuja commented Apr 16, 2024

The advantage of two words is that if you don't need to distinguish between the two you can just specify node ...

Yes. It's just whether or not we choose consistent naming over convenience.

(Also kinda makes me think I should have renamed the current op/commit label to just current in both cases)

That makes sense if we stick to the common "node" label.

@ilyagr
Copy link
Contributor

ilyagr commented Apr 17, 2024

What is the best way to use builtin_log_node? If this state continues (where it's implemented but doesn't seem turned on), it'd be nice to document.

@ilyagr
Copy link
Contributor

ilyagr commented Apr 17, 2024

Oh, I found it, you said it on Discord:

Finally submitted the potential default symbol configs, if you want to try them out on the main branch it's

[templates]
log_node = 'builtin_log_node'
op_log_node = 'builtin_op_log_node'

Update: #3517

ilyagr added a commit to ilyagr/jj that referenced this pull request Apr 17, 2024
Documents jj-vcs#3501, though
only in `templates.toml` for now (since this is mostly for
jj developers to experiment with for now).

Let me know if you prefer that I document this in config.md.
ilyagr added a commit to ilyagr/jj that referenced this pull request Apr 17, 2024
Documents jj-vcs#3501, though
only in `templates.toml` for now (since this is mostly for
jj developers to experiment with for now).

Let me know if you prefer that I document this in config.md.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants