-
-
Notifications
You must be signed in to change notification settings - Fork 655
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
Clean up CLI usage/help output, standardize flag naming #6862
Conversation
e1a41cb
to
072ad86
Compare
072ad86
to
b4257a0
Compare
While I can agree with the goal, merging this would cause a huge collapse of all of our IDE support and prevent compilation for all existing users, so I don't think that's worth the cost just for the sake of having more clean flags. Please resubmit another PR with full backward compatibility mode. |
Unless I'm missing something, this PR actually is completely backward compatible. All existing flags work as before. |
Sorry I misread, backward compatibility seems to be included. Can you confirm it's 100% compatible? |
The Haxe compiler uses stderr for output when using compiler services, so IDEs and tools will be affected by more/unexpected output there. (I believe this has been changed for Haxe 4.0, the tools have to keep backward compatibility for a looooong time, so they will be affected by a change in output.) |
I can confirm best effort + whatever CI reports, but would like to get another pair of eyes on it to be safe.
I'll drop them for now. The deprecated flags just won't show up in the usage info, and we can re-enable the warnings in the future if we choose. |
And that will confuse people who are trying to read older HXML files, and don't understand what this "undocumented" feature is. Better to leave them in the extended help |
I'm open to that, but given that the flags never differ by more than a single hyphen, I don't personally think this would be confusing. I think there's value in simplifying the help output which to me is greater than showing all deprecated flags when the mapping is obvious. |
721f836
to
add69cf
Compare
Few nitpicking: Also, I would maybe use Thoughts? |
PS: your PR also include changes to haxelib, please update it |
I like that one, I'll update. I also like
I think it's important to mimic the behavior of other tools in order to match user expectations. Looking at existing conventions, I can find (1) a lot of tools using the convention I followed above: (2) A few using only single hyphen flags: (3) C compilers, which use one hyphen for abbreviations, two elsewhere: To compare:
I would vote for the first one, it's both common and intuitive. If we intentionally want to look more like gcc that's fine too. |
Just a minor suggestion: change --dead-code-elim to --dead-code-elimination
to keep it consistent with the "full name" idea.
25 февр. 2018 г. 15:29 пользователь "Nicolas Cannasse" <
notifications@github.com> написал:
… PS: your PR also include changes to haxelib, please update it
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#6862 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADAm_JmxZMnAWEu3cRupmn66kuJLFt8rks5tYVIagaJpZM4SNOQU>
.
|
While we're at it I would do the following additional changes:
Regarding auto execution: I think we don't need -x to run Neko anymore, we should instead use the internal macro interpreter. So it kind of overlaps with
I would only keep the -x/--execute.
But that's maybe outside of the range of the current PR ;) |
Given the possibility of multiple targets with shared arguments, and --next/--each etc. I'm not sure how we would formulate this using subcommands without larger breaking changes. |
I don't mind seeing --next/--each gone. It doesn't work well with completion anyway. |
That's another good point. |
The discussion was about having something backward compatible no ? :) |
I think it is still backward compatible.
Though we can separate these into two PRs. If it has been diverged too much, I think the sub-command topic can be discussed separately. |
I pretty much disagree with all of Keven's points and I am beginning to worry :( two versions of hxml is not a good approach. --target=js stupid change for change sake! Don't indulge OCD tendencies and mess all our projects up, I use --next and I use recursive hxml, tidy flags where you can and move on to standardising the cross target toolkit build approaches, because that really is where some consistency is needed. My suggestion lock all the cross platform toolkit developers in a room at the Haxe conference and tell them they can't leave until they agree on a standard. Until that happens please don't remove stuff from hxml. |
This kind of comment doesn't help. Please focus on the content of the discussion. |
I think this is venturing out of the scope of the current PR. I'd like to keep it narrowly focused and backward compatible. For now I'll keep |
I was focused on the discussion, posx sounded good in principle, but the additional stuff would fracture the experience of day to day use so I am in favour of keeping the pr narrowly focused. ( Please before removing --next, get some stats on usage, in TrilateralBazaar I use 27 hxml files half have atleast one --next in them, and the removal proposal does not offer any alternative workflow ). |
The only thing I outright dislike here is |
Nicolas called out |
If no objections, I think this is ready to go. |
Simn noticed the xml reference in the change and it reminded me, Nodule is now more robust after I made some changes. Think I can improve parsing speed below, changing switches to nested if statements and perhaps in a few other areas. Notice although it takes longer to convert to a Nodule structure ( x3 ), it typically takes '0' time to convert to pretty custom string outputs ( s0 ) after the initial parse to Nodule, so it's ideal for quick querying. This is the trace for parsing and then converting to string this file. x0 = Xml.parse(str) took 0.10399985313415527 seconds See here ( slow to load as it's parsing lots of the native classes ) is the js demo, times vary between methods of initial parse but speed of converting nodule to pretty string is always fast. I don't know if Nodule can be improved to provide some speed in relation to XML and not looked at c++ speeds, and I am not sure if it's worth it, but I have certainly ironed out more bugs since you last saw it. If you think it's worth exploring let me know what needs to be done, and can try to improve it. Sorry I know off topic but was unsure where to post this as I think you have me on ignore for irc. Best Justin. |
This was introduced and deprecated at the same time in #6862, which seems rather pointless.
The command line is haxe -main class --interp, the extra dash in the first parm generates a usage error.
Implements parts of #6613
-
for targets or single letter flags,--
for everything else.There should be no change in behavior, although everyone will start getting compiler warnings on
-main
,-cp
etc. which are deprecated in favor of the standardized versions. The warnings go to stderr so it shouldn't impact tools consuming haxe's stdout.Example output: