Use Cell's index in the notebook as its ID

[jonathanindig]

Cell ID is now a short and we no longer read execution count. This is possible now due to the changes in #76 which eliminates our need to persist cell IDs in the order they were created.

Addresses #70 as it was due to id space collisions when reading execution count.

[jeremyrsmith]

This is really great!

One condition: if we ever decide to change the identifier type again (i.e. there's complaint about the 32,767 limit on cells in a notebook), we'll make a CellId type alias to use everywhere.

[jeremyrsmith]

It won't let me comment on the lines you didn't change 😛

Here we're creating a new (JS) NotebookCell with newCell.id which is still a string. Won't this cause inserting a cell to fail? I think the Cell classes need to be updated to deal with the numeric ID (and how to map that to an id attribute for their container element).

polynote/polynote-frontend/polynote/ui.js

Line 766 in df3637d

this.socket.send(new messages.InsertCell(path, this.globalVersion, this.localVersion++, new messages.NotebookCell(newCell.id, newCell.language, ''), current.id));

[jonathanindig]

This is really great!

One condition: if we ever decide to change the identifier type again (i.e. there's complaint about the 32,767 limit on cells in a notebook), we'll make a CellId type alias to use everywhere.

I like this idea a lot - I renamed a bunch of parameters from id => cellId but I think using a CellId type instead is a much better way to indicate what the parameter is rather than changing its name.

[jonathanindig]

@jeremyrsmith As far as I could tell this looked like the only change that needed to be done - not sure if you had anything else in mind when you said

I think the Cell classes need to be updated to deal with the numeric ID (and how to map that to an id attribute for their container element).

I wasn't able to find any other place where we explicitly do stringy things to the cell id but I definitely could've missed something.

[jeremyrsmith]

Hmm... I guess the only other issue is that the ID passed to the cell constructor also ends up as a DOM element ID. For example now we'll have something like <div id="1"> in the DOM. This isn't a legal id in HTML 4, though I guess it now is in HTML 5... but still doesn't seem like a good idea.

[jonathanindig]

ok! restored previous html ids :)