No ref name clobber

[GiovanH]

Fixes multiple bugs pointed out in #559, including

• New references clobbering each other's names
• References failing to find existing references for their link and generating a new name
• Duplicate references with the same links and names being added anyway

Resolves #559

[deathaxe]

In the following markdown

1. select the first line with an inline link
2. call "convert inline to reference"

# Test 1

[GitHub](https://github.com)

[GitHub](https://github.com)
[GitHub](https://github.com)

Restults in:

# Test 1

[GitHub][]

[GitHub][]
[GitHub](https://github.co[]//github.com
[GitHub]: )

[GitHub]: https

The tests package provides a DereferrablePanelTestCase class which uses an invisible panel as scratch pat, so different tests for real world scenarios of text manipulating functions can be implemented. I already use that to test folding and inserting task list items.

Maybe we should add tests for all new or changed functions step by step, this way.

[GiovanH]

In both master and no-ref-name-clobber, if I convert the first link to a reference, I actually get this:

# Test 1

[GitHub](https://github.com)

[GitHub](https://github.com)
[GitHub](https://github.com)

becomes

# Test 1

[GitHub][]

[GitHub](https://github.com)
[GitHub](https://github.com)

[GitHub]: https://github.com

This bug may be unrelated to what this PR addresses.

Also, I wasn't actually able to see how your test suite works; it looks like pytest, but pytest obviously can't import sublime, so I wasn't sure what the procedure was there and didn't see any documentation in either CONTRIBUTING or tests/README.

[deathaxe]

It's actually Convert All Inline Links to References which is broken. It was the only one visible when selecting the first line from the left to the right.

Even if it adds duplicates it works in current master.

---

MDE actually uses UnitTesting for both local tests and CI. It's pytest running in ST's plugin_host to be able to use all its APIs.

You are right, I should somehow write some basic docs about how to use it.

The https://github.com/SublimeText-Markdown/MarkdownEditing/blob/3.1.1/tests/test_reference_completions.py contains a quite minimal example. All the magic is packed into DereferrablePanelTestCase, which provides an invisible output panel, which is used as scratch pad. It provides methods to manipulate text and selections, read content or access self.view directly.

---

general note:

You should probably rebase your PRs onto the st3176 branch if we want to ship them for ST3 releases as well.

The master branch targets ST4, already.

[deathaxe]

I've rebased your work onto 3.1.1 branch and added a unittest.

Not sure if it's possible, but you might want to change the PR to get it merged into 3.1.1 maybe.

I am also concerned about recent syntax changes might have broken your functionality, because reference definition regions are no longer separated by a newline. Once find_by_selector returned a region, it may need to be parsed and split.

Something along those lines ...

MarkdownEditing/plugins/references.py

Lines 854 to 855 in 4e86295

	for ref in self.view.find_by_selector("meta.link.reference.def"):
	for match in self.re_reflinks.finditer(self.view.substr(ref)):

[deathaxe]

This PR seems to break auto counting of new footnote definitions when calling MarkdownEditing: New Footnote

[GiovanH]

I've rebased your work onto 3.1.1 branch and added a unittest.

Not sure if it's possible, but you might want to change the PR to get it merged into 3.1.1 maybe.

I am also concerned about recent syntax changes might have broken your functionality, because reference definition regions are no longer separated by a newline. Once find_by_selector returned a region, it may need to be parsed and split.

Something along those lines ...

MarkdownEditing/plugins/references.py

Lines 854 to 855 in 4e86295

	for ref in self.view.find_by_selector("meta.link.reference.def"):
	for match in self.re_reflinks.finditer(self.view.substr(ref)):

I am concerned about the stability of replacing pure sublime text selectors with just regular expressions. Do we have standard regex defined for the various markdown features, or is the current practice just to write them inline like getMarkers does?

[GiovanH]

Strange. After changing the row numbers to accurately match the data, the tests pass successfully on my machine, but the github test runner is now acting like the cursor is in the wrong place.

[deathaxe]

This PR contains 7403454 which is incompatible with ST3 and thus must not be part of it here. That's actually the commit which the st3176 and master branch differ by at the moment.

Actually all my commits up to 918337b must not be here.

[deathaxe]

I've dropped the "bad" commits.

[deathaxe]

I somehow start hating black. It produces different output depending on used versions.

[deathaxe]

If you are fine with my hijacked commits, I guess we may be able to merge this PR.

[GiovanH]

Looks good to me.