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

[meta] Integrate hoa/console #8

Open
Hywan opened this issue Sep 15, 2015 · 14 comments
Open

[meta] Integrate hoa/console #8

Hywan opened this issue Sep 15, 2015 · 14 comments

Comments

@Hywan
Copy link

Hywan commented Sep 15, 2015

Hello :-),

This is more a meta issue in order to discuss about your wishes to integrate Hoa\Console inside this awesome project (cf. your initial tweet and the tweet about opening an issue).

Recapitulare

Hoa\Console is more a low-level API for the terminal. First of all, every API is based on tput (aka terminal info' or terminal capabitilies). This makes everything much more compatible with the plenty of terminals we have in the wild (see Control the terminal, the right way). Based on that, we —at Hoa— have designed an API for the cursor (Hoa\Console\Cursor), the window (Hoa\Console\Window) and the mouse (Hoa\Console\Mouse). And based on that, we have designed a powerful readline (Hoa\Console\Readline), with, of course, autocompleters. Example:

Autocompleters in action

Apart from that, we have also designed an option parser and reader (resp. Hoa\Console\Parser and Hoa\Console\GetOption). They are “standalone” classes. To make it works easier with the rest of the environment, we provide Hoa\Router\Cli as another class of another library. And Hoa\Console\Dispatcher\Kit with Hoa\Dispatcher (another library) ease the development of complete commands (as defined by your project). Hoa\Router and Hoa\Dispatcher are totally optional, this must be clear.

Finally, there is also processus control with Hoa\Console\Processus: Allowing to interactivly communicate with a processus (->on('output'), ->on('input') etc.).

The documentation of Hoa\Console can be found on Hoa's website: http://hoa-project.net/Literature/Hack/Console.html.

Needs and wishes

Now, we are ready to collaborate to integrate Hoa\Console inside webmozart/console because we think this project provides a very nice high-level API. This kind of API is missing in Hoa since this is out of our goal and it can “hurt” users in some particular cases. I personally think we have a big opportunity to make a great tool here!

The only question I have is: What are your needs and wishes?

/cc @hoaproject/hoackers and especially @jubianchi who did write the Symfony bridge to Hoa\Console with the bundle. Maybe these projects can be re-used.

@Hywan
Copy link
Author

Hywan commented Jan 12, 2016

Any news?

@webmozart
Copy link
Owner

Not yet, busy with Puli atm :(

@Hywan
Copy link
Author

Hywan commented Jan 12, 2016

@webmozart Actually, I can help, but I just would like to know what you would like to do or not. Maybe the Symfony bridge to Hoa\Console would do all the trick (cc @jubianchi). Just tell me where I can start :-).

@webmozart
Copy link
Owner

Hi @Hywan! :) Late but finally some feedback. I could look a bit at Hoa\Console today and found two major concerns when it comes to an integration into Webmozart\Console.

Hoa\Console seems very tightly coupled and doesn't make a lot of use of dependency injection. For example, most classes there access the global input by calling Console::getInput(). That's fine in general, but problematic for its integration here, since here it is possible to compose console commands and call another console command from a main command into which you inject custom input (and output) streams, for example to read from a string and write to a file.

This pattern (accessing dependencies through static calls) flows through the library. We could fix it by injecting dependencies everywhere (instead of accessing them globally), but this would be very big break for your library and probably not one that you want to make.

A second "problem" (it's not a big deal for me, but for users of Webmozart\Console, hence the quotes) is the number of dependencies. The library itself has a fixed dependency on the following packages:

  • hoa/consistency
  • hoa/event
  • hoa/exception
  • hoa/file
  • hoa/protocol
  • hoa/stream
  • hoa/ustring
  • hoa/iterator (through hoa/file)

Now many of those seem useful, but I would prefer not to need some of them. For example, I don't see a big benefit in hoa/exception, hoa/protocol and hoa/event.

  • I think PHP's exception system is fine. Transactions are an entirely different topic and not covered by hoa/exception anyway.
  • I don't want to use the protocol-based event system.
  • This library is already using another event system (the Symfony event dispatcher). Having two different event systems (for the users of this library) seems a bit confusing, so it would be nice if one could be replaced for the other.

Having said all that, I think the library is great and contains great features! A bit more openness (as in the SOLID principles) would be needed to integrate it nicely though.

@Hywan
Copy link
Author

Hywan commented Aug 10, 2016

Hello @webmozart!

I know that Console::getInput and Console::getOutput as static methods is not the best but the setters also exist: Console::setInput and Console::setOutput. Inputs and outputs deal with streams, so it can be anything.
When you are calling a sub-command, you can set the new input and output, then after its execution, you can restore them. This will work. For the Hoa\Console unit test suite, we are using temporary files or string buffers for the input and output, so I know it works 😉. This is the same with Console::setTput and Console::getTput.

Hope I am not mistaken, but the Hoathis\SymfonyConsoleBridge already catch this behavior. @jubianchi may confirm this as he is the author of this package.

Would it be OK this way for the first issue?

Next, the dependencies. Hoa libraries have small dependencies and they are all re-used. For instance hoa/stream is reused by almost any stream-like libraries, like hoa/socket, hoa/websocket, hoa/file and so on. So while the list can feel “big” (I would like to double quote here because the list of dependencies for a regular product is damn huge… and I am pretty sure we are little player here, but this is out of topic), number of downloaded files are few.

I understand hoa/protocol is not necessary for you, but this is required by hoa/console to access terminfo file. The footprint of this library is tiny.

hoa/exception is required by almost every library to manage exceptions. It supports groups and transactions, see https://github.com/hoaproject/Exception#group-and-transactions. This library is a very thin layer on top of PHP exceptions. It also provides forward compatibility with next PHP versions.

hoa/event is required by hoa/console to fire events, like Hoa\Console\Processus needs for the input, output, timeout, start etc. (It extends the events from hoa/stream by the way, so complete, progress, size etc.). I understand you have symfony/event but unfortunately there is no standard interfaces so far.

So, yes, hoa/console has some dependencies. Why? Because of Hoa's quality promise and commitment. We re-use a lot of code to provide memory and speed efficiency, with a low bug presence probability. Each library is thin and addresses a specific issue. It addresses it well and seriously. Because it is thin, this is easier to ensure a strong safety and a high quality. Each library is designed to be fast with a low memory footprint.

I wonder if we can prune some dependencies with Composer. However, I don't see any library that could be pruned. hoa/exception is necessary if an exception is thrown. hoa/event is necessary for stream-related and console-related events/listeners. hoa/protocol is necessary to access terminfo files. hoa/stream is necessary to read terminfo files, for the input and the output of the console etc. We could prune hoa/iterator. And we could prune hoa/ustring if you are not likely to use the default readline or the getoption parser (but this is a 2 classes library).

If I sum the files that are going to be loaded and to run, we have… (do some additions): 3 from hoa/consistency + 6 from hoa/event + 3 from hoa/file + 4 from hoa/protocol + 1 from hoa/stream (+/- 2) = 17 files. Only 17 classes to get everything working on all platforms and on all PHP versions. Is it too much?

Let me think if we could reduce the number of dependencies but this is not going to be easy.

@djmattyg007
Copy link
Contributor

It's not necessarily about the number of classes, it's about the size of the dependency tree. Each additional package is a package I have to keep track of for updates (ie security). Also, each additional package is one that could potentially conflict with something else I may want to install. For example, one package may depend on one version of a package, and another package may depend on a different version of the same package, or one of its dependencies, or something like that.

Also, a specific note: it appears that you have a circular dependency between hoa/exception & hoa/event, and also hoa/exception & hoa/consistency. This doesn't fill me with confidence when choosing a library.

It's also difficult for me to see the point of packages like hoa/consistency and hoa/exception. As someone putting together an application from many different components, most of which won't be depending on hoa/exception, I'm feel as though I'm unlikely to get much benefit from it, and am therefore likely to avoid anything that would bring it in. hoa/consistency has its own issues - it supposedly backports a bunch of functionality to earlier, now unsupported, versions of PHP (that developers should be actively discouraged from using), but it has a hard requirement on PHP 5.5 or above. This doesn't particularly make sense.

All of these components that I, as a developer using PHP 7.0, see no real need for, and would therefore generally avoid any package that would bring these in as dependencies.

@Pierozi
Copy link

Pierozi commented Aug 12, 2016

hoa/exception provide a great features to manage Exception much smarter than only try, catch.
And yes, this library have his own repository, because there is the quality of code, keep minimal effort with maximum of re-usability.

Even if hoa\consistency backports couples functionality, she's also have some crucial features for lot of others libraries, like the Xcallable class.

There's are crucial tools for improve quality and stability of others libraries.
If theses two repositories was inside hoa/console you should never noticed.

About the versions package dependence and upgrade, there's are not conflict with our Rush Release Versioning

We had troubles couples months ago with BC related to Hoa\Core but with smart tag updates on all libraries concerned we can also avoid BC break dependencies for end user.

From my personal opinion, each Hoa Library must be focus on the purpose of what she`s build for.
This is much easier to read and understand for new developer, and the code can be keep small.
I think is a reason of the quality of each library. Even if that produce circular dependency, it's not a problem because each repository can be much easier to improve individually.

In the past, we had Hoa\Core which been split into 4 repositories for improve quality, performance and stability, you can report to this issues for understand why.

@djmattyg007
Copy link
Contributor

hoa/consistency and hoa/exception may provide great functionality, but the point is that most non-hoa code aren't going to use these libraries, and automatically bringing them in as hard dependencies isn't desirable due to exactly this problem. I may want the functionality of hoa/console, but that doesn't mean I also want the Xcallable class, for example.

Also, I obviously wasn't explicit enough about my point regarding hoa/consistency. The fact that it is attempting to polyfill functionality for PHP versions where it is no longer necessary to do so (due to it only supporting PHP 5.5 and above) shows me that you have a serious problem at some level. I don't know if it's an organisational issue, a time issue, a general code quality issue, or something else entirely. The point is that it says to me "don't use this code".

@tgalopin
Copy link
Contributor

I think this issue is not about natively integrating hoa Console in webmozart Console but to provide a bridge between them. Am I wrong?

@webmozart
Copy link
Owner

@tgalopin The initial idea here was indeed to use hoa/console as foundation for this package

@tgalopin
Copy link
Contributor

Okay. Then I agree with @djmattyg007 on the optionnal part: even if the hoa libraries as lightweight, I think most of the users won't need the low-level features they provide. I think the best way to achieve a full integration (not just a bridge) is to create an extended version of webmozart/console (you could say a different, more advanced "distrbution").

@Pierozi
Copy link

Pierozi commented Aug 12, 2016

@djmattyg007 maybe you don't want play with low level functionality, but hoa\console need xcallable for working anyway as readline use in https://github.com/hoaproject/Console/blob/master/Readline/Readline.php#L157-L169

If you consider hoa/consistency as a polyfill you have clearly wrong understand the code, in this way I invite you to open issues for asking details or tell us what you consider as low quality or can be improved.

@djmattyg007
Copy link
Contributor

djmattyg007 commented Aug 12, 2016

hoa/consistency attempts to polyfill the trait_exists function from PHP 5.4, despite requiring PHP 5.5. This is a clear WTF that rings alarm bells about what other obviously absurd pieces of code are lying about in this package, or in other packages brought in as a dependency by hoa/console.

Perhaps you should reflect on the idea that a developer probably isn't going to choose hoa/console because it depends on a subjectively excellent exception management library. Rather, they're going to choose hoa/console because of the functionality in that package alone. If hoa/console or one of its dependencies contains code that goes against the ideals of the developer, they are less likely to choose your package over the competition. The more dependencies your package has, the more likely they are to not choose it.

@Hywan
Copy link
Author

Hywan commented Aug 15, 2016

I think we should calm down a little bit :-).

@djmattyg007 There is no organisational issue or something else. Good catch for trait_exists, it lands since PHP 5.4 and so we can drop the polyfill (an issue has been opened, hoaproject/Consistency#14).

I tend to agree with @Pierozi when he is saying that if most of the libraries were embedded inside hoa/console, you would probably never noticed :-/. If a dependency includes 50 classes, it would be less “noticeable” than 10 dependencies including 5 classes. Good or bad? Don't know. This is the current fashion. Period.

Hoa can greatly benefit of your constraints and your code analysis. Thank you for this. We are not foolproof at all (e.g. with trait_exists).

Since several days, I am reading the code tracking if a dependency or two could be removed or not. So far, each one is required for good reasons, but I am continuing this work.

I am wrong if I say that only hoa/exception and hoa/consistency are the culprits, or are they other dependencies?

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

No branches or pull requests

5 participants