Skip to content
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

Keep track of line maps for "tr_open" tokens inside tbodys #705

Closed
fabiospampinato opened this issue Aug 29, 2020 · 8 comments
Closed

Keep track of line maps for "tr_open" tokens inside tbodys #705

fabiospampinato opened this issue Aug 29, 2020 · 8 comments

Comments

@fabiospampinato
Copy link

fabiospampinato commented Aug 29, 2020

It looks like "tr_open" tokens have their "map" property populated only if they are inside the "thead" of a table, while "tr_open" tokens inside the "tbody" section of the table don't have their "map" property populated.

It would be useful if all "tr_open" tokens got their "map" properties populated so that scroll synchronization solutions that are built on top of that could reliably map table rows too.

@fabiospampinato
Copy link
Author

Same for image tokens, should I open a separate issue about that?

@puzrin
Copy link
Member

puzrin commented Aug 29, 2020

Mapping [rows, but without columns] is for blocks only (images are inline).

tr_open seems possible to extend https://github.com/markdown-it/markdown-it/blob/master/lib/rules_block/table.js#L175 with token.map = [ nextLine, nextLine + 1 ];

@fabiospampinato
Copy link
Author

fabiospampinato commented Aug 29, 2020

Mapping [rows, but without columns] is for blocks only (images are inline).

Right, especially since images can have arbitrary heights it would still be useful to be able to map them back to the source line when more than one image is found within the same paragraph. Otherwise I'd imagine synchronized scrolling would become somewhat unreliable under those scenarios.

tr_open seems possible to extend https://github.com/markdown-it/markdown-it/blob/master/lib/rules_block/table.js#L175 with token.map = [ nextLine, nextLine + 1 ];

Yeah that should be trivial. Would you like me to submit a PR for that?

@puzrin
Copy link
Member

puzrin commented Aug 30, 2020

Otherwise I'd imagine synchronized scrolling would become somewhat unreliable under those scenarios.

Since big images are usually inside separate paragraph, result is good enougth. Anyway, sourcemapping of inlines is different level of complexity and not planned.

Yeah that should be trivial. Would you like me to submit a PR for that?

As you wish. I hope, we will solve all pending issues at once next week.

@rlidwka
Copy link
Member

rlidwka commented Sep 11, 2020

Fixed, looks like these were missed by accident.

@fabiospampinato
Copy link
Author

@rlidwka Actually line mapping have been added to td_open tokens, not tr_open tokens, was that intentional? Shouldn't tr_open tokens have line mapping provided for too?

@rlidwka rlidwka reopened this Sep 25, 2020
rlidwka added a commit that referenced this issue Sep 25, 2020
 - `table`, `tbody`, `tr` now have mapping
 - `th`, `td`, `inline` in tables do not have it

close #705
rlidwka added a commit that referenced this issue Sep 25, 2020
 - `table`, `tbody`, `tr` now have mapping
 - `th`, `td`, `inline` in tables do not have it

close #705
rlidwka added a commit that referenced this issue Sep 25, 2020
 - `table`, `tbody`, `tr` now have mapping
 - `th`, `td`, `inline` in tables do not have it

close #705
rlidwka added a commit that referenced this issue Sep 25, 2020
 - `table`, `tbody`, `tr` now have mapping
 - `th`, `td`, `inline` in tables do not have it

close #705
@rlidwka
Copy link
Member

rlidwka commented Sep 25, 2020

not tr_open tokens, was that intentional

Not intentional, fixed in 9fe835b.

@fabiospampinato
Copy link
Author

Awesome, thank you!

chrisjsewell pushed a commit to executablebooks/markdown-it-py that referenced this issue Jan 14, 2021
This commit updates the port to be up-to-date with markdown-it v11.0.1 (2020-09-14), applying two fixes:

- Fix blockquote lazy newlines, [[#696](markdown-it/markdown-it#696)].
- Fix missed mappings for table rows, [[#705](markdown-it/markdown-it#705)].
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants