-
Notifications
You must be signed in to change notification settings - Fork 476
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
Display recipe aliases inline with the recipe doc #2342
base: master
Are you sure you want to change the base?
Display recipe aliases inline with the recipe doc #2342
Conversation
1daa64f
to
0673479
Compare
I personally think this is an improvement, but I'd like to get some feedback on it first. Paging some regulars: @laniakea64 @neunenak @runeimp What do you think? |
I agree that displaying aliases inline would be nicer, and I like the |
I think that may make it look somewhat odd to have aliases next to the recipe signature. Putting the aliases there would mean increasing the width of the recipe signature, causing all recipe docs to have to have a lot of padding between them. I think that suggested notation works best for flags/short flags. Whereas there's no limitation on how short or long, or how many, your aliases are. The way I see it is, an alias is not pertinent information to be prominently displayed in the listing. The list is still useful, even if you have to search slightly for your commands alias. But maybe my thinking is wrong here :) I had originally colored the |
I think changing the color would be nice, so that it doesn't look like it's part of the doc. |
Think semantically: Invocation info is on the left, and description of what the recipe does is on the right. Mixing this information together makes it less human-parseable. As a compromise to avoid the formatting issue, the aliases could be on the left side of the documentation comment and colorized:
|
I think the downside is that it pushes the doc to the right, and the doc is probably more interesting to the user than the list of aliases. |
I love the concept of inlining aliases. I may have suggested it at one point but don't remember. In any case there is on glaring problem (that I hate) which is that a recipe could have infinite aliases. The current method is the only way that handles the situation consistently. But if we know (do we know?) that if a recipe has an alias at all that say a single alias is used 97% of the time and two aliases 2% of the time and three or more aliases only 1% of the time then I believe there is wiggle room to inline by default. We could also do the inlining by default and have a |
Are there any opinions on what color it should be, or a scheme the project is adhering to?
Is this something really to be concerned with, though? You also can have as long as a recipe doc as you want, spanning multiple lines. Wouldn't it be up to the end user to decide how many aliases is too many, like it's their decision on how long the recipe doc is?
So with |
a964041
to
4e05250
Compare
Maybe a good compromise would be to inline the aliases if the total length of them is short, and otherwise stick with the current behavior of listing them below the recipe they are aliases for? (I actually think it would be a good idea to indent the alias lines below the recipe line, to make it visually clearer that the lines form a conceptual group). I think the most common case is definitely that people have one short alias for a recipe, so optimizing for that case is ok as long as it doesn't completely break things for people with less common use cases. And in that common case having an inline alias makes sense. And having a command line flag to force aliases onto their own line in all cases also makes sense to me. I agree that having a special color for aliases is a good idea, and I don't feel strongly about whether an inlined alias comes before or after the doc comment. |
4e05250
to
cee8997
Compare
One concern: Are there possibly cases where the alias is actually the descriptive, user-discoverable name, and it would hurt discoverability if it isn't present in the list output on its own line? |
Trying this out on that justfile: yes, moving the aliases to the right of the documentation comment hurts discoverability. If the aliases were inlined to the left of the documentation comment, it would actually improve discoverability compared to having the aliases on their own lines. |
cee8997
to
87f1752
Compare
I've updated the code so that the Here's how it looks in my terminal: For recipe's that have aliases but do not have a document, should the aliases display as As an aside, the inspiration for this notation (aliases on the right of the command help text) came from This is how clap handles many and long aliases for comparison:
And with Just using these changes (other recipes removed for brevity):
So this follows the pattern clap is using pretty closely, but obviously Just is not clap so it doesn't necessarily hold much weight on the decisions made here. But I Just wanted to show that this is an established practice in other tools.
I'm not sure I'm understanding the concern here. I looked through #1725 and the referenced discussion, and it seems like aliases are being used as a workaround to what looks to be a bug in argument expansion when passing to dependencies? |
Nice, purple is good choice! 👍 It's distinct from doc comments, distinct from recipe parameters and values, and keeps contrast with the blue doc comment when using a very warm monitor color temperature with Gammastep.
That clap output has different semantics from
When the main recipe name is uninformative: if the descriptive alias isn't in the invocation info, people can't scan |
Yes, knowing what percentage of the user base this would directly affect lets us know if we can make changes with abandon or how much consideration we should consider for potential disruption, if that information is available. It also can inform the context of reasonable defaults to avoid unnecessary configuration options which potentially adds complexity with no benefit to the end user. I generally prefer control. But I use Mac because I like the defaults of the OS. I could get more control with POSIX systems like GNU/Linux. But I'm not in love with most of the defaults.
Right, |
87f1752
to
e120daa
Compare
Just pushed a commit that adds a I'm not sure if it makes sense for this to be a Justfile setting, however, as the documentation reads:
And this setting wouldn't control interpretation or execution, just the display of the EDIT: Just wanted to point out this will need some additional tests but I figured I'd hold off until there's some decisions made on how this should progress forward. |
What is the big hangup over having a setting? My default recipe is Also when I do run |
I was just going off of what the documentation states, so as not to conflict with the design decisions of Just. However, Just options can also be set via environment variables. So you could add |
Because this option doesn't decide how the justfile will behave. It only decides how
And the global environment variable suggestion seems much better anyway, as it lets you have your preference on this regardless of whether you authored the justfile. |
@marcaddeo Sorry, if I was a bit aggressive. It's just been a very rough few months for me and I was feeling a bit attacked with that prior statement. The environment variable is not a 100% solution for me. My problem is easily fixed by simply adding a setting I can set differently in every other project as I need instead of an all or nothing 100% of the time. I can do a hack like adding @casey could you weigh in? I don't want to go against your general plans for Just but this exception seems reasonable to me. |
@runeimp It feels like there maybe a XY-like problem here: To avoid further misunderstanding of where you're coming from, could you please be more specific what exactly you would like to achieve that you think can only be done by a justfile setting to toggle inlining of aliases? |
I disagree. I do consider the display of information a behavior of the tool.
@laniakea64 I appreciate that your considering this is an XY-problem. And this could be, but in the sense that this feature should not have been an option only to begin with. I think that it should have instead been a setting first with the option added in support of the possible transitive nature of the setting in any given situation. The issue is that I will want it one way for over 90% of my projects. And I will 100% want it the other way for a small percentages of projects. A setting via an environment variable only allows for one way for all local projects. And some special consideration on how to manage that setting in the other projects to overcome the environment variable. The only way to handle that which is consistent with anything is a setting I can manage per project. I can set the environment variable if needed for the majority of projects and then consistently set the local setting in the Justfile for unambiguous handling in the other projects. Or I end up special case handling for every project I realize has to be handled differently because a simple setting is not desired. |
Thanks for clarifying. So without a setting, with only a command-line flag + environment variable, the closest would be every project having its own recipe to run Would there be a pattern in when you'd want this one way vs the other? Could/should |
I doubt a universal rational for when to and when not to is achievable. More than one alias? That doesn't mean much if all the aliases are one or two letters. Any alias more than 8 characters? Lots of people will think that is too many or not enough of a limit. Only when the terminal has over 80 characters wide? Basically the same problem as max length of a single alias. What font is being used in the terminal? etc. All of that and more is likely way too fiddly. I suspect the only way to make such a formula work at all would require several more settings. I think the binary |
Good catch! I haven't played with documentation blocks yet so completely forgot about them in the discussion. |
I'm still quite conflicted about this. On the one hand, I think in the common case it looks better. On the other hand, when there are many aliases, it's not great. I don't like the idea of a setting, since this feels like a user preference thing, and not necessarily a justfile author thing, and the justfile author can include a default recipe with I also think I like it on the right, not a super strong preference, but it mirrors Does that all sound reasonable? |
As noted in #2342 (comment) , it doesn't mirror What about instead of a boolean inline/no-inline, having a tri-state option:
To me, it's not really about preference: there are personal reasons why inlining aliases on the left would make Setting a global environment variable (that can be overriden on the command line if needed) is fine with me. |
I agree my general use case can be easily fixed manually by just adjusting the recipe where needed. But a default setting in a tool documents the preference for the style of the As this is really a personal preference thing I think a setting makes sense. Having the aliases on the left of the description is a UX thing. Objectively I think we can agree that on the left makes logical sense in that, how you use the option, is effectively in one place at a glance. But people who don't care if aliases exist or not, because they have amazing memories or simply don't care about such things, won't care much if they are listed at all. Both options can be accomplished with a setting. None of this is a deal breaker for me. I'm just voicing for my preference and the flexibility other users may also find very satisfying. As a front-end developer for most of my early dev career, I've always put a premium on UX. 😇 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments.
7bd6243
to
d19878a
Compare
@casey all feedback should be addressed, let me know if you'd like to see any other changes. I also added a test for the new |
@casey also just pushed a commit to add If you think this is getting too granular, I can revert that commit, but figured it was pretty easy to implement that as well now that the logic was shifted from your feedback. |
I think that since there are a bunch of different ways to display aliases, the best way is with an option which takes a value, instead of multiple flags. The option should be something like Is I'm also not sure about the option names. |
+1 for What about naming values like:
|
2bc314e
to
3b31db6
Compare
Alrighty, I switched it up to an |
3b31db6
to
fed57db
Compare
fed57db
to
4505816
Compare
Hey @casey, is there anything else you'd like changed in this PR? Would love to get it merged soon. If we think the changes are stable, I can update the relevant documentation to match. |
This updates the
--list
subcommand to display aliases inline with the recipe doc.Before:
After: