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

use message arrays with new status-logger API #514

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@joehand
Member

joehand commented Sep 7, 2016

@juliangruber's recent PR got me thinking about how to improve the logger API (which was not very clear before). I updated the status-logger API to print arrays of messages and have the user be responsible for updating the arrays.

This PR has two logging lists, messages and progressLines. messages we push and never update. progressLines we update & overwrite.

Does this API makes more sense on the logger?

The status-logger update also uses ansi-diff-stream so updates are a bit smoother looking.

@juliangruber

This comment has been minimized.

Show comment
Hide comment
@juliangruber

juliangruber Sep 7, 2016

Collaborator

Great idea to use a persistent data type for log lines! I was thinking it could be even simpler with an api like this maybe?

var lines = []
var log = logger(lines, { ... })

lines[0] = 'Starting dat'
lines[1] = 'Connecting...'
lines.push('Downloading in dir')

// it's easier to just work with one array

or even

var log = logger()
log.set(0, 'Starting' dat)
log.set(1, 'Connecting...')
log.push('Downloading in dir')

// there's only one thing you work with,
// downside: need to mirror parts of array api

or with optional titles

var log = logger()
log.set('starting', 0, 'Starting' dat)
log.set('connecting', 1, 'Connecting...')
log.push('Downloading in dir')

log.remove('connecting')

// so you work less with indexes

What do you think?

Collaborator

juliangruber commented Sep 7, 2016

Great idea to use a persistent data type for log lines! I was thinking it could be even simpler with an api like this maybe?

var lines = []
var log = logger(lines, { ... })

lines[0] = 'Starting dat'
lines[1] = 'Connecting...'
lines.push('Downloading in dir')

// it's easier to just work with one array

or even

var log = logger()
log.set(0, 'Starting' dat)
log.set(1, 'Connecting...')
log.push('Downloading in dir')

// there's only one thing you work with,
// downside: need to mirror parts of array api

or with optional titles

var log = logger()
log.set('starting', 0, 'Starting' dat)
log.set('connecting', 1, 'Connecting...')
log.push('Downloading in dir')

log.remove('connecting')

// so you work less with indexes

What do you think?

@joehand

This comment has been minimized.

Show comment
Hide comment
@joehand

joehand Sep 7, 2016

Member

I kind of like the title or one array idea but trying to figure out how it'd work in practice. @juliangruber what does log.push() do differently?

Seems like the title idea would look something like (just thinking through the current use case):

// Dat is initialized
log.set('directory', 0, 'Starting dat') // Will show "Sharing Dir"
log.set('link', 1, '') // Will show link
log.set('progress', 2, '')  // will be progress bar
log.set('swarm', 3, '\nConnecting...') // will be swarm info + upload/download speed

Would print:

Starting Dat

Connecting...

Then when we get the key and start adding, the result we want to print is:

Sharing /Users/joe/node_modules/dat/tests/fixtures

Share Link: 10942ad84d5d6ab69e74bdca955d6f2a7c37c4d7107e7ab57897fe61c3ddb587
The Share Link is secret and only those you share it with will be able to get the files

[====> ] Progress

Waiting for connections.

So you'd need something like:

log.update('directory', 'Sharing Dir')
log.update('link', 'Share Link: <link> etc')
log.update('progress', '[====> ] Progress')
log.update('swarm', '\nWaiting for connections.')

This seems nice to not worry about the index but also a bit constricted making it harder to change things down the line.

Member

joehand commented Sep 7, 2016

I kind of like the title or one array idea but trying to figure out how it'd work in practice. @juliangruber what does log.push() do differently?

Seems like the title idea would look something like (just thinking through the current use case):

// Dat is initialized
log.set('directory', 0, 'Starting dat') // Will show "Sharing Dir"
log.set('link', 1, '') // Will show link
log.set('progress', 2, '')  // will be progress bar
log.set('swarm', 3, '\nConnecting...') // will be swarm info + upload/download speed

Would print:

Starting Dat

Connecting...

Then when we get the key and start adding, the result we want to print is:

Sharing /Users/joe/node_modules/dat/tests/fixtures

Share Link: 10942ad84d5d6ab69e74bdca955d6f2a7c37c4d7107e7ab57897fe61c3ddb587
The Share Link is secret and only those you share it with will be able to get the files

[====> ] Progress

Waiting for connections.

So you'd need something like:

log.update('directory', 'Sharing Dir')
log.update('link', 'Share Link: <link> etc')
log.update('progress', '[====> ] Progress')
log.update('swarm', '\nWaiting for connections.')

This seems nice to not worry about the index but also a bit constricted making it harder to change things down the line.

@joehand

This comment has been minimized.

Show comment
Hide comment
@joehand

joehand Sep 7, 2016

Member

To add, I think either of your ideas would work for our current use case. But the original reason I had two arrays was for printing stuff above progress. We were essentially doing array.splice() repeatedly, so the index of the progress line, for example, would keep changing in a single array.

There were basically two output sections:

message[0] = [Done] foo.txt added
message[1] = [Done] File.txt added
#---------- section split
progress[0] = Sharing Folder

progress[1] = [====> ] Progress

progress[2] = Connected to 11 peers. Uploading 20000 mb/s.

With two arrays, we wouldn't have to keep track of changing indices. We essentially have two buckets to keep track of. This mattered more when we were listing filenames but less so now. But when we add the list feature or any other feature where we have more than 5 lines, a single index may be too hard to keep track of.

The current API of status-logger takes a single array, so we could just go for that and see how it works. Then go back to two if it gets complicated.

Member

joehand commented Sep 7, 2016

To add, I think either of your ideas would work for our current use case. But the original reason I had two arrays was for printing stuff above progress. We were essentially doing array.splice() repeatedly, so the index of the progress line, for example, would keep changing in a single array.

There were basically two output sections:

message[0] = [Done] foo.txt added
message[1] = [Done] File.txt added
#---------- section split
progress[0] = Sharing Folder

progress[1] = [====> ] Progress

progress[2] = Connected to 11 peers. Uploading 20000 mb/s.

With two arrays, we wouldn't have to keep track of changing indices. We essentially have two buckets to keep track of. This mattered more when we were listing filenames but less so now. But when we add the list feature or any other feature where we have more than 5 lines, a single index may be too hard to keep track of.

The current API of status-logger takes a single array, so we could just go for that and see how it works. Then go back to two if it gets complicated.

@juliangruber

This comment has been minimized.

Show comment
Hide comment
@juliangruber

juliangruber Sep 8, 2016

Collaborator

@juliangruber what does log.push() do differently?

it would just forward to the internal array, like log.prototype.push = function(line){ this._lines.push(line) }.

This seems nice to not worry about the index but also a bit constricted making it harder to change things down the line.

If you already know where this will fail, let's not do it, otherwise I'd say let's at least try.

To add, I think either of your ideas would work for our current use case. But the original reason I had two arrays was for printing stuff above progress. We were essentially doing array.splice() repeatedly, so the index of the progress line, for example, would keep changing in a single array.

What about multi dimensional arrays then?

var lines = []
var log = logger(lines)
var progress = lines[0] = []
var swarm = lines[1] = []

progress.push('foobar')

So the logger lib would simply flatten the log arrays, no matter how deep they are. I think I'd prefer this, or the 'title' solution.

Collaborator

juliangruber commented Sep 8, 2016

@juliangruber what does log.push() do differently?

it would just forward to the internal array, like log.prototype.push = function(line){ this._lines.push(line) }.

This seems nice to not worry about the index but also a bit constricted making it harder to change things down the line.

If you already know where this will fail, let's not do it, otherwise I'd say let's at least try.

To add, I think either of your ideas would work for our current use case. But the original reason I had two arrays was for printing stuff above progress. We were essentially doing array.splice() repeatedly, so the index of the progress line, for example, would keep changing in a single array.

What about multi dimensional arrays then?

var lines = []
var log = logger(lines)
var progress = lines[0] = []
var swarm = lines[1] = []

progress.push('foobar')

So the logger lib would simply flatten the log arrays, no matter how deep they are. I think I'd prefer this, or the 'title' solution.

@joehand joehand closed this Nov 4, 2016

@joehand joehand deleted the update-logger branch Nov 4, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment