Skip to content

Commit

Permalink
Fix spacer remove when out of viewport
Browse files Browse the repository at this point in the history
Add test proving it is broken

Add some delay before scrolling to the bottom

Make the test more obvious
  • Loading branch information
fcollonval committed Nov 7, 2023
1 parent f5927a1 commit eae4c1f
Show file tree
Hide file tree
Showing 6 changed files with 604 additions and 25 deletions.
95 changes: 70 additions & 25 deletions packages/nbdime/src/common/mergeview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1695,6 +1695,26 @@ export class MergeView extends Panel {

const sumDeltas = new Array(editors.length).fill(0);
const nLines = editors.map(editor => editor.state.doc.lines);
let processViewportAlignement = false;
const viewports = editors.map(e => ({

Check warning on line 1699 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1698-L1699

Added lines #L1698 - L1699 were not covered by tests
from: offsetToPos(e.state.doc, e.viewport.from),
to: offsetToPos(e.state.doc, e.viewport.to),
}));

const addStartSpacers = () => {
sumDeltas.forEach((delta, i) => {
const from = editors[i].viewport.from;
builders[i].add(

Check warning on line 1707 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1704-L1707

Added lines #L1704 - L1707 were not covered by tests
from,
from,
Decoration.widget({
widget: new PaddingWidget(delta * lineHeight),
block: true,
side: -1,
}),
);
});
};

for (const alignment_ of linesToAlign) {
const alignment = this._showBase ? alignment_.slice(0, 3) : alignment_;
Expand All @@ -1709,41 +1729,66 @@ export class MergeView extends Panel {
: Math.min(...lineDeltas.slice(1));
const correctedDeltas = lineDeltas.map(line => line - minDelta);

correctedDeltas.forEach((delta, i) => {
// Don't compute anything for the base editor if it is hidden
if (!this._showBase && i === 0) {
return;
const afterViewport = alignment.map((a, i) => a > viewports[i].to.line);

Check warning on line 1732 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1732

Added line #L1732 was not covered by tests
if (!afterViewport.some(after => !after)) {
// All follow-up alignments are after the viewport => bail out
break;

Check warning on line 1735 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1735

Added line #L1735 was not covered by tests
}

const beforeViewport = alignment.map(
(a, i) => a < viewports[i].from.line,

Check warning on line 1739 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1738-L1739

Added lines #L1738 - L1739 were not covered by tests
);

if (beforeViewport.some(before => !before)) {
if (!processViewportAlignement) {
// First time we add spacer in the viewport => add initial spacers
addStartSpacers();

Check warning on line 1745 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1745

Added line #L1745 was not covered by tests
}
// Alignments are zero-based
let line = alignment[i];
processViewportAlignement = true;

Check warning on line 1747 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1747

Added line #L1747 was not covered by tests

if (delta > 0 && line < nLines[i]) {
sumDeltas[i] += delta;
correctedDeltas.forEach((delta, i) => {

Check warning on line 1749 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1749

Added line #L1749 was not covered by tests
// Don't compute anything for the base editor if it is hidden
if (!this._showBase && i === 0) {
return;

Check warning on line 1752 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1752

Added line #L1752 was not covered by tests
}
const line = alignment[i];

Check warning on line 1754 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1754

Added line #L1754 was not covered by tests

// This method include the correction from zero-based lines to one-based lines
const offset = posToOffset(editors[i].state.doc, {
line,
column: 0,
});
if (line < nLines[i]) {
sumDeltas[i] += delta;

Check warning on line 1757 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1757

Added line #L1757 was not covered by tests

builders[i].add(
offset,
offset,
Decoration.widget({
widget: new PaddingWidget(delta * lineHeight),
block: true,
side: -1,
}),
);
}
});
const offset = posToOffset(editors[i].state.doc, {

Check warning on line 1759 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1759

Added line #L1759 was not covered by tests
line,
column: 0,
});

builders[i].add(

Check warning on line 1764 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1764

Added line #L1764 was not covered by tests
offset,
offset,
Decoration.widget({
widget: new PaddingWidget(delta * lineHeight),
block: true,
side: -1,
}),
);
}
});
} else {
correctedDeltas.forEach((delta, i) => {
sumDeltas[i] += delta;

Check warning on line 1777 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1775-L1777

Added lines #L1775 - L1777 were not covered by tests
});
}
}

if (!processViewportAlignement) {
// There are no spacer in the viewport, we still have to add top spacer
addStartSpacers();

Check warning on line 1784 in packages/nbdime/src/common/mergeview.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/common/mergeview.ts#L1784

Added line #L1784 was not covered by tests
}

// Padding at the last line of the editor
const totalHeight = nLines.map((line, i) => line + sumDeltas[i]);
const maxHeight = Math.max(...totalHeight);
totalHeight.slice(0, this._showBase ? 3 : 4).forEach((line, i) => {
if (maxHeight > line) {
if (maxHeight > line && viewports[i].to.line === nLines[i]) {
const end = editors[i].state.doc.length;
const delta = maxHeight - line;
sumDeltas[i] += delta;
Expand Down
250 changes: 250 additions & 0 deletions ui-tests/data/diff_test4/center.ipynb
Original file line number Diff line number Diff line change
@@ -0,0 +1,250 @@
{
"cells": [
{
"cell_type": "code",
"execution_count": null,
"id": "eabfe4e0-d646-4d09-95a4-09ee3d030db6",
"metadata": {},
"outputs": [],
"source": [
"import numpy as np\n",
"import matplotlib.pyplot as plt\n",
"\n",
"def gaussian(x, a, b, c):\n",
" #calculate a gaussian\n",
" return a * np.exp(-b * (x-c)**2)\n",
"\n",
"def sinus ():\n",
" # Here you can see a sinus function\n",
" nx = 100\n",
" x = np.linspace(-5.0, 5.0, nx)\n",
" y = np.sin(x)\n",
" return x, y\n",
"class NbdimeConfigurable(HasTraits):\n",
"\n",
" def configured_traits(self, cls):\n",
" traits = cls.class_own_traits(config=True)\n",
" c = {}\n",
" for name, _ in traits.items():\n",
" c[name] = getattr(self, name)\n",
" return c\n",
"\n",
"\n",
"_config_cache = {}\n",
"def config_instance(cls):\n",
" if cls in _config_cache:\n",
" return _config_cache[cls]\n",
" instance = _config_cache[cls] = cls()\n",
" return instance\n",
"\n",
"\n",
"def _load_config_files(basefilename, path=None):\n",
" \"\"\"Load config files (json) by filename and path.\n",
"\n",
" yield each config object in turn.\n",
" \"\"\"\n",
"\n",
" if not isinstance(path, list):\n",
" path = [path]\n",
" for path in path[::-1]:\n",
" # path list is in descending priority order, so load files backwards:\n",
" loader = JSONFileConfigLoader(basefilename+'.json', path=path)\n",
" config = None\n",
" try:\n",
" config = loader.load_config()\n",
" except ConfigFileNotFound:\n",
" pass\n",
" if config:\n",
" yield config\n",
"\n",
"\n",
"def recursive_update(target, new, include_none):\n",
" \"\"\"Recursively update one dictionary using another.\n",
" \"\"\"\n",
" for k, v in new.items():\n",
" if isinstance(v, dict):\n",
" if k not in target:\n",
" target[k] = {}\n",
" recursive_update(target[k], v, include_none)\n",
" if not include_none and not target[k]:\n",
" # Prune empty subdicts\n",
" del target[k]\n",
"\n",
" elif not include_none and v is None:\n",
" target.pop(k, None)\n",
"\n",
" else:\n",
" target[k] = v\n",
"\n",
"\n",
"def build_config(entrypoint, include_none=False):\n",
" if entrypoint not in entrypoint_configurables:\n",
" raise ValueError('Config for entrypoint name %r is not defined! Accepted values are %r.' % (\n",
" entrypoint, list(entrypoint_configurables.keys())\n",
" ))\n",
"\n",
" # Get config from disk:\n",
" disk_config = {}\n",
" path = jupyter_config_path()\n",
" path.insert(0, os.getcwd())\n",
" for c in _load_config_files('nbdime_config', path=path):\n",
" recursive_update(disk_config, c, include_none)\n",
"\n",
" config = {}\n",
" configurable = entrypoint_configurables[entrypoint]\n",
" for c in reversed(configurable.mro()):\n",
" if issubclass(c, NbdimeConfigurable):\n",
" recursive_update(config, config_instance(c).configured_traits(c), include_none)\n",
" if (c.__name__ in disk_config):\n",
" recursive_update(config, disk_config[c.__name__], include_none)\n",
"\n",
" return config\n",
"\n",
"\n",
"def get_defaults_for_argparse(entrypoint):\n",
" return build_config(entrypoint)\n",
"\n",
"\n",
"class Global(NbdimeConfigurable):\n",
"\n",
" log_level = Enum(\n",
" ('DEBUG', 'INFO', 'WARN', 'ERROR', 'CRITICAL'),\n",
" 'INFO',\n",
" help=\"Set the log level by name.\",\n",
" ).tag(config=True)\n",
"\n",
"\n",
"class Web(NbdimeConfigurable):\n",
"\n",
" port = Integer(\n",
" 0,\n",
" help=\"specify the port you want the server to run on. Default is 0 (random).\",\n",
" ).tag(config=True)\n",
"\n",
" ip = Unicode(\n",
" '127.0.0.1',\n",
" help=\"specify the interface to listen to for the web server. \"\n",
" \"NOTE: Setting this to anything other than 127.0.0.1/localhost \"\n",
" \"might comprimise the security of your computer. Use with care!\",\n",
" ).tag(config=True)\n",
"\n",
" base_url = Unicode(\n",
" '/', help=\"The base URL prefix under which to run the web app\",\n",
" ).tag(config=True)\n",
"\n",
" browser = Unicode(\n",
" None,\n",
" allow_none=True,\n",
" help=\"specify the browser to use, to override the system default.\",\n",
" ).tag(config=True)\n",
"\n",
" persist = Bool(\n",
" False, help=\"prevent server shutting down on remote close request \"\n",
" \"(when these would normally be supported).\",\n",
" ).tag(config=True)\n",
"\n",
" workdirectory = Unicode(\n",
" default_value=os.path.abspath(os.path.curdir),\n",
" help=\"specify the working directory you want \"\n",
" \"the server to run from. Default is the \"\n",
" \"actual cwd at program start.\",\n",
" ).tag(config=True)\n",
"\n",
"\n",
"class WebTool(Web):\n",
" pass\n",
"\n",
"\n",
"\n",
"class IgnoreConfig(Dict):\n",
"\n",
" def validate_elements(self, obj, value):\n",
" value = super(IgnoreConfig, self).validate_elements(obj, value)\n",
" for k, v in value.items():\n",
" if not k.startswith('/'):\n",
" raise TraitError('ignore config paths need to start with `/`')\n",
" if not (v in (True, False) or\n",
" (isinstance(v, (tuple, list, set)) and\n",
" all(isinstance(i, str) for i in v)\n",
" )):\n",
" raise TraitError('ignore config value needs to be True, False or a list of strings')\n",
" return self.klass(value)\n",
"\n",
"\n",
"class _Ignorables(NbdimeConfigurable):\n",
"\n",
" sources = Bool(\n",
" None,\n",
" allow_none=True,\n",
" help=\"process/ignore sources.\",\n",
" ).tag(config=True)\n",
"\n",
" outputs = Bool(\n",
" None,\n",
" allow_none=True,\n",
" help=\"process/ignore outputs.\",\n",
" ).tag(config=True)\n",
"\n",
" metadata = Bool(\n",
" None,\n",
" allow_none=True,\n",
" help=\"process/ignore metadata.\",\n",
" ).tag(config=True)\n",
"\n",
" id = Bool(\n",
" None,\n",
" allow_none=True,\n",
" help=\"process/ignore identifiers.\",\n",
" ).tag(config=True)\n",
"\n",
" attachments = Bool(\n",
" None,\n",
" allow_none=True,\n",
" help=\"process/ignore attachments.\",\n",
" ).tag(config=True)\n",
"\n",
" details = Bool(\n",
" None,\n",
" allow_none=True,\n",
" help=\"process/ignore details not covered by other options.\",\n",
" ).tag(config=True)\n",
"\n",
" Ignore = IgnoreConfig(\n",
" default_value={},\n",
" help=\"a custom ignore config\"\n",
" ).tag(config=True)\n",
"\n",
"\n",
"def noisy_gaussian():\n",
" # gaussian array y in interval -5 <= x <= 5\n",
" nx = 100\n",
" x = np.linspace(-5.0, 5.0, nx)\n",
" y = gaussian(x, a=2.0, b=0.5, c=1.5)\n",
" noise = np.random.normal(0.0, 0.2, nx)\n",
" y += noise\n",
" return x, y"
]
}
],
"metadata": {
"kernelspec": {
"display_name": "Python 3 (ipykernel)",
"language": "python",
"name": "python3"
},
"language_info": {
"codemirror_mode": {
"name": "ipython",
"version": 3
},
"file_extension": ".py",
"mimetype": "text/x-python",
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.11.3"
}
},
"nbformat": 4,
"nbformat_minor": 5
}

0 comments on commit eae4c1f

Please sign in to comment.