search

fcrespo82 / atom-markdown-table-formatter

A simple markdown plugin to format tables.
MIT License
21 stars 7 forks source link

When the table starts in the first column of the first line the plugin adds some duplication to the table #23

Closed fcrespo82 closed 8 years ago

fcrespo82 commented 8 years ago 
|Left align|Right align|Center align|
|:---------|----------:|:----------:|
|This|This|This|
|column|column|column|
|will|will|will|
|be|be|be|
|left|right|center|
|aligned|aligned|aligned|

is formatted as

| Left align | Right align | Center align |
|:-----------|------------:|:------------:|
| This       |        This |     This     |
| column     |      column |    column    |
| will       |        will |     will     |
| be         |          be |      be      |
| left       |       right |    center    |
| aligned    |     aligned |   aligned    |
|        will |     will     |
| be         |          be |      be      |
| left       |       right |    center    |
| aligned    |     aligned |   aligned    |

It appears that when then table is the first thing in the document that the myIterator runs twice.

fcrespo82 commented 8 years ago

Changing line 75 from editor.backwardScanInBufferRange(@regex, range, myIterator) to editor.scanInBufferRange(@regex, range, myIterator) seens to fix. But needs more tests.

This can be testes by checking out the ColumnAsFirstItem branch.

@lierdakil can you help me with that?

lierdakil commented 8 years ago

I will take a look. Bear in mind that I'm insanely busy right now, so it may take a day or two for me to get to this.

fcrespo82 commented 8 years ago

Don't worry. Take your time... I'm not in a rush and I find it rather difficult to someone use a table as the first thing in the document.

On Tue, Dec 15, 2015 at 10:36 AM Nikolay Yakimov notifications@github.com wrote:

I will take a look. Bear in mind that I'm insanely busy right now, so it may take a day or two for me to get to this.

— Reply to this email directly or view it on GitHub https://github.com/fcrespo82/atom-markdown-table-formatter/issues/23#issuecomment-164751291 .

Fernando Xavier de Freitas Crespo

lierdakil commented 8 years ago

Looks like this goes deeper than our little package. I've opened an issue on Atom's repo. Let's see what Atom devs say first.

fcrespo82 commented 8 years ago

I thought that too. Because the ranges were being reported correctly. Let's see what they will say.

Fernando X. F. Crespo

Em 15 de dez de 2015, às 16:49, Nikolay Yakimov notifications@github.com escreveu:

Looks like this goes deeper than our little package. I've opened an issue on Atom's repo. Let's see what Atom devs say first.

— Reply to this email directly or view it on GitHub.

lierdakil commented 8 years ago

So, it's a bug in Atom core after all. Fix is available, and will likely get into next minor release. https://github.com/atom/text-buffer/pull/120

That said, we don't have to scan backwards, I don't think. I've changed testing code to provide more coverage (also to save some scrolling) and added more editor-specific tests. You can find it in editor-tests branch. ColumnAsFirstItem is merged into that one already.

fcrespo82 commented 8 years ago

@lierdakil you're great, thank you very much for your help. There's some sense of achievement finding a bug in such a great product as Atom.

I thought we should use backwards scan not to mess with marker positions after inserting the formatting spaces, but appears as Atom already takes care of this.

fcrespo82 commented 8 years ago

Closed by #25