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

WANTED: cross-platform maintainers (Windows, OSX, TypeScript, etc) #164

Open
cronvel opened this issue Mar 30, 2021 · 35 comments
Open

WANTED: cross-platform maintainers (Windows, OSX, TypeScript, etc) #164

cronvel opened this issue Mar 30, 2021 · 35 comments

Comments

@cronvel
Copy link
Owner

cronvel commented Mar 30, 2021

This lib is becoming huge and while I can keep up with the core mechanism, it's become harder and harder to support various OS and the ever-growing list of terminals. Also since I don't own Windows or OSX, I can't help much.

I'm also searching a TypeScript maintainer for the types, since I'm a vanilla JS Node coder, and I have no time to learn that at the moment.

Finally, a little help with the API documentation would be fine too (a native speaker, or someone very fluent with US english).

That's why I'm officially searching for maintainers, and I will keep the list of active one on this issue.
Feel free to comment here if you are wanting to help.

Windows maintainers:

OSX maintainers:

TypeScript maintainers:

Documentation maintainers:

  • @Abstract-Programming
@cronvel cronvel pinned this issue Mar 30, 2021
@cronvel
Copy link
Owner Author

cronvel commented Mar 30, 2021

Possible maintainers that helped the project in the past:

If you are interested, please chime in! (even if you are not listed!)

@cronvel cronvel changed the title WANTED: cross-platform maintainer (Windows, OSX, TypeScript, etc) WANTED: cross-platform maintainers (Windows, OSX, TypeScript, etc) Mar 30, 2021
@katsanva
Copy link

@cronvel I can take care of TypeScript & OSX issues for sure

@cronvel
Copy link
Owner Author

cronvel commented Mar 30, 2021

@katsanva Great! Welcome aboard!

@dangilkerson
Copy link
Contributor

@cronvel I can help out with windows and osx maintenance. I can also be a backup for typescript if needed.

@NuSkooler
Copy link

@cronvel My work is all under Linux so I'm not sure I can be much help in the Windows arena.

@katnova
Copy link

katnova commented Mar 31, 2021

@cronvel I speak English natively, and would be willing to help out with maintaining documentation.

@cronvel
Copy link
Owner Author

cronvel commented Mar 31, 2021

@dangilkerson @Abstract-Programming Great! I added you to the official list of maintainers!

@NuSkooler Ok, I thought you were working on both OS, my misunderstanding ;)

@NuSkooler
Copy link

@cronvel Yeah just Linux. With that said if I can help in some way I'd like to.

@cronvel
Copy link
Owner Author

cronvel commented Apr 1, 2021

@NuSkooler Ok, let me know. Maybe maintainer for BBS? Or whatever you like.

@Digicrat
Copy link

I just came across this package, and am definitely liking it so far. It's certainly much easier to use than the old and unmaintained blessed.js library, even if it can't quite match all of it's functionality just yet.

I'd like to say that I'll help out with maintaining this project, but realistically I know that I won't have time to do much more than perhaps addressing any bugs (or trivial missing features) I might encounter as I start using it. On that vein, I just created a PR for adding row selection functionality to TextTable.

I will offer a few friendly suggestions for the project:

  • Github recently added a 'Discussions' capability. You might want to enable that in the project Options to help separate discussions/questions from issues in the tracker.
  • Use JSDoc syntax to embed documentation in the code, then run the jsdoc util to export HTML documentation of the API to be hosted via Github pages (with exported files pushed to a dedicated docs-only branch). That should help create the most complete documentation with minimal effort, and makes it easy to create/update the documentation as the library is updated.
  • Long-term, I'd suggest migrating the code to ES6 style class { ... } syntax. That is of course functionally equivalent to the existing code, but imho easier to read, document and hence maintain in the long-term (and I do hope this project remains active for a long time).

@cronvel
Copy link
Owner Author

cronvel commented Apr 28, 2021

@Digicrat The project will remain active, but I could not work full time on it, because it obviously doesn't pay my bill! However it's a project I like, and it's my sole project that get some public attention at the moment, so it is important to me, especially as a freelance developer, to push it further, it's a sort of business card in a sense...

However, I can only improve it on my free time, since it's hardly part of my daily job, so it is usually only improved by short burst once in a while.

@pwilkowski
Copy link

@katsanva Would it be possible to export typescript definitions once more? Or at least share how did you do it? I've tried dts-gen however without success. Library is awesome however TS definitions are very old and does not support things like tables.

@katsanva
Copy link

@pwilkowski will definitely do it when I got a moment

@pwilkowski
Copy link

@katsanva I was trying to do it with dts-gen however I got this error:

Unexpected crash! Please log a bug with the commandline you specified.
***\node_modules\dts-gen\bin\lib\run.js:125
        throw e;
        ^

TypeError: Object prototype may only be an Object or null: undefined
    at Function.create (<anonymous>)
    at Object.<anonymous> (***\terminal-kit\lib\document\Button.js:155:27)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (***\terminal-kit\lib\document\Slider.js:32:16)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)

I assume you extracted this manually?

@katsanva
Copy link

@pwilkowski yes, exactly

@pwilkowski
Copy link

Wouldn't it be better at this point to just update JSDoc in project itself so that it can be automatically exported? I am not that versed in this process yet but I could try and update it to make whole process lot easier.

@katsanva
Copy link

@pwilkowski AFAIK no JSDoc here, at least comments, JSDoc-like in the best case

@pwilkowski
Copy link

pwilkowski commented May 18, 2021

I know, but I could try to write them and then generate typings from it. Shouldn't be that hard, just might take some time. I like this lib so I thought I could make it bit better.

This could also help to generate docs from it.

to put it simply, implement what @Digicrat suggested like this:

@pwilkowski
Copy link

@cronvel what do you think about it? If I will have green light from you I can generate docs such as:

Which gives support to native JS and auto docs:

@cronvel
Copy link
Owner Author

cronvel commented May 18, 2021

@pwilkowski Is it something on my end that makes dts-gen crash? I don't see anyreason why an object would have undefined as its prototype in the Button class.

@pwilkowski
Copy link

@cronvel unfortunately I have no clue, this was the first time i tried to use dts-gen.
It fails on this line:

Button.prototype = Object.create( Text.prototype ) ;

in Button.js

According to what I have read it might be circular dependency however I was not able to see any.
I have been trying with this: https://github.com/Microsoft/dts-gen

@cronvel
Copy link
Owner Author

cronvel commented May 18, 2021

@pwilkowski (cross-posted) It looks nice, does the syntax support indentation? For example if one argument is an object of option, is it possible to indent nested properties?

@pwilkowski
Copy link

@cronvel If you are asking if you can put indent before @param tag then yes, it is possible since JSDoc just ignores indents in code (AFAIR) like this:

/**
 * It displays a bar representing the value. It uses unicode characters to improve the precision.
 *
 * @param {number} value the value to display as bar
 * @param {Object} [options]
 *		@param {number} [options.innerSize] the inner width in characters (default: 10)
 *		@param {function} [options.barStyle] the style of the bar, default to term.blue
 *		@param {boolean} [options.str] if true it outputs nothing, instead it returns a string (default: false)
 *
 * @returns {Terminal|string}
 */

unless you meant something else?

@cronvel
Copy link
Owner Author

cronvel commented May 18, 2021

@pwilkowski Nice, that exactly what I was asking.

The only thing that is bothering me is the mix of JSDoc vs manual doc.
There is already a rather big doc, and I don't think it would be a good idea to migrate it to source code.

Are you using JSDoc already on a big project? Is it ok to split the doc? On one side an API-only (+ 1 or 2 lines of short description) JSDoc, and on the other side a complete manual documentation with long description and examples.

@pwilkowski
Copy link

pwilkowski commented May 18, 2021

@cronvel Yes I am using JSDoc in big internal project

There are several advantages over writing docs manually:

  1. Whenever you write a new method or change existing one, you only need to update it in one place - above function definition, then you generate docs automatically.
  2. It is a lot easier to maintain library as whole syntax of docs is generated.
  3. It is easier to navigate and create navigation (like links to functions in description) since that part is also done automatically

To put it simply ALL the annoying logistics of creating docs are gone, your docs are in pretty standard format and are easy to read.
Also you can chose the template to your liking:
like this one: http://clenemt.github.io/docdash/ or this one:
https://github.com/SoftwareBrothers/better-docs

HOWEVER

this lib is pretty mature so for now I would suggest to first create JSDoc itself. That alone gives method autocompletion and IDE documentation with no extra steps.

Then after we could confirm it's working, try to create typings from it, should be easy and typings would no longer be necessairy.

Then after that generate docs, and for time being keep two versions of docs. Docs for examples are needed anyways unless there is a way to autogenerate them also - I am not expert in that regard.

I have created a fork and pushed some examples here:
https://github.com/pwilkowski/terminal-kit/blob/master/lib/Palette.js

@pwilkowski
Copy link

I have made some significant progress on forked repo, was able to extract typings and add a lots of JSDoc comments, still nowhere near done but i will be working bit by bit. So far almost no code changes (just one tiny bit in gpm.js) For example Rect.ts is fairly done.

@cronvel
Copy link
Owner Author

cronvel commented May 19, 2021

@pwilkowski I appreciate your commitment, but please, avoid modifying the code, I have little time ATM, so it would delay the merge of the PR if I have to review/test all those changes. Particularly in the gpm code. I you think you found a bug, create a separated PR for that.

Also, you could probably speed up the whole process if you create only doc for public API methods. I apologize because it's not always clear if a method is internal or public, but sometime there is a // internal comment above the method.

@pwilkowski
Copy link

pwilkowski commented May 19, 2021

@cronvel unfortunately I have encountered some issue. While comments themselves seems to be somewhat working for documentation BUT the whole process of exporting typings from that fails because of "lazy" modules. I have tried multiple solutions found on stackoverflow yet none of them work.

The "quickfix" for that was replacing lazy properties and module.exports with es6 syntax:

so instead of:

const termkit = {} ;
module.exports = termkit ;
//
lazy.requireProperty( termkit , 'tty' , './tty.js' ) ;

this:

export default termkit;
export { termkit };
//
import { tty_ } from './tty';
termkit.tty = tty_ ;

However that would possibly break library

Some background:
Currently I am using this lib for custom App that has quite some CLI actions going on. Basically interacting with database, creating users, etc.
There is NO alternative to this library.
Other libs are either: too simple (like just coloring text), super outdated, does not have even half of the options of this one.

Right now I am leaning towards making a copy of your library, from a scratch, in typescript. Technically MIT license allows that but whenever I can publish the copy or not should be your permission.

To put it simply I intend to: Create new library with new name, copy parts of your code, translate it to typescript.
And if you agree, make it public.
EDIT: On a second thought I would just keep it private and import only what I need

@cronvel
Copy link
Owner Author

cronvel commented May 20, 2021

@pwilkowski Erf... This typescript thing is so nitpicky. So, the files work independently, but the connection is lost between anywhere things are lazy loaded? Right? Is it possible to add some hinting with a comment, like one would do for ESlint?

Lazy-loading considerably speed up startup, and many users only use a very small part of the lib, e.g. just output text with colors. If there is no work-around for typescript, it could be possible to duplicate the entry-point file and remove lazy loading for it.
E.g instead of require('terminal-kit') someone wanting typing would use require('terminal-kit/lib/no-lazy.js').
Still it would be best if we could avoid such duplication.

IMHO, forking would not be a good thing, because it would split effort and community, and there is no disagreement between us at the moment. But like you said, the license permit it.

@katsanva
Copy link

katsanva commented May 20, 2021

@pwilkowski what about something like

import { Tty, TermKit } from './interfaces';
import * as Lazyness from 'lazyness';
const lazy = new Lazyness(require);
const termkit: TermKit = {
  tty: lazy.require<Tty>( './tty.js' )
};

export const tty = termkit.tty;

export default termkit;

such es6 module migration is anyway a breaking change so it is actually fine to change the exports

Btw, cant it be made with JSDoc in a next way?

/**
@typedef {Object} TerminalKit
@property {import('./tty')} tty 
*/

/**
@type {TerminalKit}
*/
const termkit = {}
...

@pwilkowski
Copy link

@cronvel the thing is about typescript is that its suppose to be strict.
that being said es6 module imports has been a thing for a while and things such as lazy loading has been pretty much built in.

for example instead of creating one global variable that hold literally everything in library, you just export it separately.

You just

import {terminal} from './terminal';
import {tty} from './module2';

terminal.red.bgRed('something');
tty('something else')

(require works too)

this way, if something in core requires a module it will be loaded anyway but everything else such as LabeledInput, if not used in code, won't be loaded at all. And if someone uses packager like webpack, it will get treeshaken so it won't even be in compiled source code. Without any additional effort at all.

This is some good reference of what I am talking about. https://www.freecodecamp.org/news/how-to-use-es6-modules-and-why-theyre-important-a9b20b480773/

that being said, converting code to es6 import/exports might be breaking change, so my first thought would be to make terminal-kit2. However after thoughtfully inspecting a code I realized even something like this would be a massive amount of work. Unfortunately I do not have that much spare time. So for my own purpose I have created a private lib, made from scratch with bits of borrowed code - just enough to make my team life work easier in creating complex cli commands. I could make it public but as for now it doesn't even have 10% of what your lib can do. It is also completely incompatible.

@katsanva
If you are talking about writing definitions by hand then it will work. But I just could not get them to work with tsc generator.
I tried various different things including example you have provided but termkit.d.ts was just empty file.

SUMMARY:
For now I do not have answer for everyone. Instinct tells me the next logical step would be to create a terminal-kit successor from scratch with code borrowed from original lib. Simply because at this point it would be extremely hard trying to modernize existing version and result would be incompatible anyways. BUT this endeavor is too much for a single person. So I will not try to do it unless a number of other people would be also willing to. And original intent to update docs died down when it turned out it won't help exporting typescript typings (or it would also take massive amount of hacks).

@cronvel
Copy link
Owner Author

cronvel commented May 22, 2021

@pwilkowski I don't think it would be that hard to "modernize" this lib. Except for ES6 module which was not very-well supported by Node.js until recently, the lib has almost no technical debt, which is great for a lib that old. And ES6 modules are not an issue at all except for TypeScript (web packager are meaningless for a CLI-oriented Node.js lib).

Don't be such a pessimist! ;)

Tell me the tools you are using with the dedicated command you are running, and I will look into it once I got some coding free time. Maybe it will require scripting some glue code, but I don't think it will be that hard.

My only concern is about the chaining call hack (e.g. term.red.italic.bold('some string')), I'm pretty sure it will be hard, if possible at all, to build typings for that.

@pwilkowski
Copy link

At first i thought it would be easy as well, I even tried to change the code. Until I realized that pretty much everything would have to be changed in order to make it more "modular". The thing is. terminal-kit is centralized (has only one code output point). Trying to make it more independent would simply break backwards compatibility.

consider the following:

Centralized approach:

const term = require('terminal-kit');
term.spinner(...);
term.progressBar(...);

Modular approach

const spinner = require('terminal-kit/spinner');
const progressBar = require('terminal-kit/progressBar');
spinner(...);
progressBar(...);

So unless there is a way to make tsc read properly how the code is structured I do not see any other solution.

Web Packagers are not meaningless when it comes to libraries as well. While they might be called "WEB" packagers, their advantage is that they can basically automate all transpilation processes be it typescript->js or js->js. And in my case it is a pretty big application that is not only CLI. CLI is just a part of it, there is also a webserver, socket server, cron, migration and cli.

To put it simply packagers like webpack or rollup can create compiled/optimized/minified library from any source code (js/coffe/ts) and make UMD/CommonJS/whatever modules.

That being said, packager is not required here.

Have to stress that out.

As for tools, its in my fork, specifically:
this entire file: https://github.com/pwilkowski/terminal-kit/blob/master/tsconfig.json
and those lines in package.json https://github.com/pwilkowski/terminal-kit/blob/master/package.json#L22-L24

after installing packages, just tsc in console and typescript should generate typings in types folder. Its not entire setup for typescript but we can worry about that once (IF) you could find a way to document the methods/modules.

@cronvel
Copy link
Owner Author

cronvel commented May 24, 2021

Please continue this discussion on that dedicated TypeScript issue.
This issue is about project maintainers.
It looks like Github has no feature to move comments from an issue to another... :/

@juliuszfedyk
Copy link

I would love to help with the documentation. I also have a mac and windows machines available.

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

8 participants