jupyterlab-contrib / jupyterlab-vim

Vim notebook cell bindings for JupyterLab
https://jupyterlab-contrib.github.io/jupyterlab-vim.html
MIT License
662 stars 43 forks source link

Correctly save column position when moving by lines #28

Closed ianhi closed 3 years ago

ianhi commented 3 years ago

Fixes: https://github.com/axelfahy/jupyterlab-vim/issues/23

Binder link: https://mybinder.org/v2/gh/ianhi/jupyterlab-vim/markdown-cols?urlpath=lab

Here is a test snippet:

#include "syscalls.h"
/* getchar:  simple buffered version */
int getchar(void)
{
  static char buf[BUFSIZ];
  static char *bufp = buf;
  static int n = 0;
  if (n == 0) {  /* buffer is empty */

    n = read(0, buf, sizeof buf);
    bufp = buf;
  }
  return (--n >= 0) ? (unsigned char) *bufp++ : EOF;
}

new behavior

cols-fixed

old (broken): cols-broken

This Pulls in the updated version from the codemirror source https://github.com/codemirror/CodeMirror/blob/9d0f9d19de70abe817e8b8e161034fbd3f907030/keymap/vim.js#L1922

The new version checks for the functions in the switch so I had to pull out the moveByLines function and give it a name. Other than that I just tweaked how the switch works and added some new lines from the codemirror version.

cc @BrendanMartin and @lukashergt

ianhi commented 3 years ago

closing and reopening to see if I can get that binder bot to comment.

lukashergt commented 3 years ago

closing and reopening to see if I can get that binder bot to comment.

Not sure whether this helps here, but I've found empty commits useful for similar situations:

git commit --allow-empty -m "wakey wakey GitHub Actions"
ianhi commented 3 years ago

Here's the binder link: https://mybinder.org/v2/gh/ianhi/jupyterlab-vim/markdown-cols?urlpath=lab

ianhi commented 3 years ago

I think that the builds are failing because the package.json scripts use jlpm which is jupyterlabs shipped version of yarn, but the build env doesn't install jlab. So there are a few solutions:

  1. Install jupyterlab in the build env
  2. switch the package.json scripts to use npm

Do you have a preference for either?

Also, are there meant to be automated version bumps for the python - if so I don't think that those are working.

ianhi commented 3 years ago

Also, are there meant to be automated version bumps for the python - if so I don't think that those are working.

Nevermind I was confused, vresion.py uses the package.json i see.

axelfahy commented 3 years ago

I think that the builds are failing because the package.json scripts use jlpm which is jupyterlabs shipped version of yarn, but the build env doesn't install jlab. So there are a few solutions:

1. Install jupyterlab in the build env

2. switch the package.json scripts to use npm

Do you have a preference for either?

I think you are right about jlpm. I don't have any preference, what do you think is cleaner?

ianhi commented 3 years ago

I think you are right about jlpm. I don't have any preference, what do you think is cleaner?

Let's just also install jlab, I think that might be necessary anyway as build:prod calls jupyter labextension build .

ianhi commented 3 years ago

oh hmm, but that makes the npm publish action need jlab as well...

But keeping jlpm will keep us consistent with the cookiecutter which I think is a positive. Do you have anything against combining the two publish actions?

ianhi commented 3 years ago

Actually I'm even more confused, because the build requirments in pyproject.toml really ought to have picked up jlab as a dep :/

ianhi commented 3 years ago

aaah, that's because python setup.py apparently doesn't look inside of pyproject.toml, if we use the fairly new python -m build instead I think it will work.

ianhi commented 3 years ago

@axelfahy I think that should fix it, but theres no way to test without merging first, so lets go for it?

axelfahy commented 3 years ago

@ianhi Yes, let's go for it then, I'll merge.

axelfahy commented 3 years ago

@ianhi By Is it okay to remove the action to publish to npm? Without it, the version installed using the extension manager (directly from jupyterlab) will not be updated, right?

ianhi commented 3 years ago

I added that action into the the single publish action - it should publish to both pypi and npm.