Skip to content

Add support for iteratively inserting array elements into a formatted message and printing the result#526

Merged
kgryte merged 14 commits into
developfrom
feature/array-log
Dec 28, 2022
Merged

Add support for iteratively inserting array elements into a formatted message and printing the result#526
kgryte merged 14 commits into
developfrom
feature/array-log

Conversation

@steff456

Copy link
Copy Markdown
Contributor

Description

What is the purpose of this pull request?

This pull request:

  • Adds the log package that allows pretty printing collections of the same length
  • If a scalar value and a collection are provided, it will broadcast to each item in the collection.

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.


@stdlib-js/reviewers

@steff456 steff456 added the Feature Issue or pull request for adding a new feature. label Sep 22, 2022
@steff456 steff456 self-assigned this Sep 22, 2022
Comment thread lib/node_modules/@stdlib/array/log/examples/index.js Outdated
Comment thread lib/node_modules/@stdlib/array/log/examples/index.js Outdated
Comment thread lib/node_modules/@stdlib/array/log/lib/logger.js
Comment thread lib/node_modules/@stdlib/array/log/lib/logger.js Outdated
Comment thread lib/node_modules/@stdlib/array/log/test/test.js Outdated
Comment thread lib/node_modules/@stdlib/array/log/test/test.js Outdated
Comment thread lib/node_modules/@stdlib/array/log/test/test.js Outdated
Comment thread lib/node_modules/@stdlib/array/log/test/test.js Outdated
Comment thread lib/node_modules/@stdlib/array/log/test/test.js Outdated
Comment thread lib/node_modules/@stdlib/array/log/test/test.js Outdated
Comment thread lib/node_modules/@stdlib/array/log/test/test.js Outdated
Comment thread lib/node_modules/@stdlib/array/log/test/test.js Outdated
Comment thread lib/node_modules/@stdlib/array/log/test/test.js Outdated
Comment thread lib/node_modules/@stdlib/array/log/test/test.js Outdated

@kgryte kgryte left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@steff456 Thanks for submitting this PR! I believe we still need to add repl.txt and TypeScript declarations. Once those are added, this PR should be ready for merge.

@kgryte kgryte changed the title Add support for pretty printing collections of the same length Add support for iteratively inserting array elements into a formatted message and printing the result Sep 22, 2022
Comment thread lib/node_modules/@stdlib/array/log/docs/repl.txt
Comment thread lib/node_modules/@stdlib/array/log/docs/repl.txt Outdated
Comment thread lib/node_modules/@stdlib/array/log/docs/repl.txt Outdated
Comment thread lib/node_modules/@stdlib/array/log/docs/repl.txt
Comment thread lib/node_modules/@stdlib/array/log/docs/types/index.d.ts Outdated
Comment thread lib/node_modules/@stdlib/array/log/docs/types/index.d.ts Outdated
@kgryte

kgryte commented Sep 29, 2022

Copy link
Copy Markdown
Member

@Planeshifter Any thoughts on the package name, here?

@Planeshifter

Copy link
Copy Markdown
Member

@stdlib/array/log with variable name log seems fine with me. Could use logArray. If we were concerned about log being confused with logarithm, another alternative may be print?

@kgryte

kgryte commented Sep 30, 2022

Copy link
Copy Markdown
Member

@Planeshifter another question is whether this package belongs in @stdlib/array.

Comment thread lib/node_modules/@stdlib/array/log/docs/repl.txt
Comment thread lib/node_modules/@stdlib/array/log/docs/types/index.d.ts Outdated
Comment thread lib/node_modules/@stdlib/array/log/docs/types/test.ts Outdated
@kgryte

kgryte commented Dec 11, 2022

Copy link
Copy Markdown
Member

After thinking about this a bit more, I am thinking that we should create a new top-level console namespace (i.e., @stdlib/console) for housing this and other console related packages. In which case, this package could be moved to @stdlib/console/log-array (or @stdlib/console/log-each; I think this name is better).

Having a console namespace could be useful, as there is enough variation among console implementations in JS engines that having consistent behavior (i.e., a set of standard APIs) could be useful. These APIs would largely be independent of our planned printf set of APIs.

Prior art

  • console-browserify: especially among older engines, having a consistent console API motivated Browserify and other bundlers to ship an interface consistent with what existed in Node.js.

Considerations

  • In JS engines, console is a global instance. For APIs supporting labels, these are stored in a global cache (e.g., count, group, etc). We could provide a small enhancement, which would be similar to debug and export a factory method along side the global instance. A factory instance could then maintain its own cache in order to avoid collisions. Individual packages such as console/log, console/error, etc, would simply be wrappers around methods:

    function log() {
        console.log.apply( null, arguments );
    }

    With factories, we'd put the above in a closure and to a bit of magic to make labels work (e.g., console.log( '<label>:%s', msg )) and ensure that the output appeared the same.

Thoughts, @Planeshifter?

@Planeshifter

Copy link
Copy Markdown
Member

@kgryte I think we should go ahead and create a top-level console namespace!

@kgryte

kgryte commented Dec 14, 2022

Copy link
Copy Markdown
Member

Cool. I'll add the top-level namespace. @steff456 Once I do, we'll want to rename the package in this PR. I'll circle back once we have everything in place.

@steff456

Copy link
Copy Markdown
Contributor Author

Ok! I'll wait for the signal 😉

@kgryte

kgryte commented Dec 15, 2022

Copy link
Copy Markdown
Member

@steff456 The console namespace has now been created. Would you mind renaming this package to

@stdlib/console/log-each

and updating the alias from log to logEach?

@kgryte

kgryte commented Dec 15, 2022

Copy link
Copy Markdown
Member

You'll need to merge in the latest commits from develop.

Comment thread lib/node_modules/@stdlib/console/log-each/README.md Outdated
Comment thread lib/node_modules/@stdlib/console/log-each/lib/logger.js
Comment thread lib/node_modules/@stdlib/console/log-each/lib/logger.js Outdated
Comment thread lib/node_modules/@stdlib/console/log-each/package.json Outdated
Comment thread lib/node_modules/@stdlib/console/log-each/README.md Outdated

@kgryte kgryte left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! Thanks, @steff456!

@kgryte
kgryte merged commit f5d2c96 into develop Dec 28, 2022
@kgryte
kgryte deleted the feature/array-log branch December 28, 2022 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature Issue or pull request for adding a new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants