tajmone / ST4-Asciidoctor

AsciiDoc Package for SublimeText 4
https://tajmone.github.io/ST4-Asciidoctor
MIT License
11 stars 6 forks source link

Syntax scope tweaks (heading.level, list.unordered, and list.ordered) #50

Open polyglot-jones opened 6 months ago

polyglot-jones commented 6 months ago

Fixed the syntax for ordered and unordered lists by changing punctuation.definition.list_item to just punctuation.definition.list (no _item) and adding .ordered or .unordered -- to be consistent with labeled lists.

Reverted markup.heading.0-5.asciidoc back to markup.heading.level.0-5.asciidoc (i.e. added .level back in)

Fixed AsciiDoc Dark's colorizing of block titles.

tajmone commented 6 months ago

@polyglot-jones, I've added you as a collaborator so that you'd have more freedom to change, edit and improve the project.

If you think that these changes are for the better, I'm fine with it.

The only thing I ask you is that you contribute each changes-set as single commits — if you need to amend a commit (e.g. because you forgot something) then amend the commit, rather than adding a second commit that fixes the previous one. In the dev branch we have free to force-rewrite history, if the need be, but this is something that we need to agree upon before doing it, since it might damage branches the other was working on, so we should only do it to preserve a clean and linear repository history. Each commit should represent a specific change-set, in a way that checking out that commit doesn't lead to a broken-state of the package. That's really all it boils down to.

This PR contains two separate commits, so the question is whether they should be squashed into a single commit (i.e. one is just a fix for the other) or whether they are intentionally kept separate because they represent two distinct and independent change-sets (although they might be related, they are independent steps). I leave that to you — i.e. when you're OK with it, just push to the dev branch.

Honestly, many of the scope names where just inherited from the original AsciiDoc package — which, BTW was written a very long time ago, when TextMate like syntaxes were still fairly new and scoping naming conventions were experimental at best. Today we have a better grasp of proper scoping conventions, since we have the benefit to explore how editors like ST, VSCode, TextMate, Atom, and others, have tackled the various syntaxes.

As far as scope names are concerned, usually details matter mostly in the context of advances editing features one is planning to add to a package, not just syntax highlighting. I tend to add the extra scope segment at the end just because in the future it might be good to be able to discern different constructs when building plug-ins and other scripted extensions, although in terms of syntax highlighting they might not even be used — but we're still far from that goal right now, so we might as well handle these details when the time comes.

Most likely, as the package develops we'll be faced with new challenges, and even after-thoughts, and end up revising scope names according to new needs. So, it's not entirely unexpected that we might "change the changes" from time to time. In fact, there's really no other way to achieve our goal except by daring and experimenting, which of course some times might mean we make choices that later on we retract.

polyglot-jones commented 6 months ago

The only thing I ask you is that you contribute each changes-set as single commits

The problem is I pushed the first commit up to GitHub (my fork) before I noticed that I forgot the tests. If I had caught the issue before the push, I certainly would have just amended/rebased/squashed the prior commit. If there is away to do a rebase after a push and then push the rebase cleanly, I am not aware. When I've tried it in the past it always ends up with ugly merge commits that just make a hash of things. Note: I use TortoiseGit on Windows for 99% of my git work, and CLI for 1%. I haven't spent any time looking into what VSCode or GitHub's desktop app can do that's better than Tortoise. So, while I wholeheartedly agree that atomic commits are desirable, I have zero spare time to invest in researching new tools solely in the hopes of learning how to squash commits post-push. It's, frankly, the least of my worries.

tajmone commented 6 months ago

The problem is I pushed the first commit up to GitHub (my fork) before I noticed that I forgot the tests. If I had caught the issue before the push, I certainly would have just amended/rebased/squashed the prior commit.

A pull request is just a branch, so you're free to tweak the commits history locally and force push again the same branch associated to the commit. Also, you don't need to stick to the PR, you can just squash your changes to dev and push dev to the repository, since you have privileges to do so.

If there is away to do a rebase after a push and then push the rebase cleanly, I am not aware.

As far as pull requests go, the creator of the PR can force push his PR branch again: GitHub associates the PR with the incoming branch (i.e. the contributor) so, by design, it allows room for changes. All such operations are on your side, i.e. GitHub only cares about the source branch (which is on your fork or clone, depending on where you're pushing from).

I use TortoiseGit on Windows for 99% of my git work, and CLI for 1%. I haven't spent any time looking into what VSCode or GitHub's desktop app can do that's better than Tortoise.

As far as I know, TortoiseGit should be just fine for such operations, although I have no idea on which menu does what — I had installed it briefly in the past, so I don't remember much. In Sublime Merge, after staging, the same button that is used to commit has the extra option "Amend previous commit", and if you select a commit in the UI and click the right button you see a menu "squash with parent" — it would be reasonable to expect something similar with other tools too.

Basically, amending a previous commit is done by staging the new changes, and instead of creating a new commit you amend the last commit. But you can also squash together multiple commits together, as long as they are chained together. And, of course, you can always amend the commit(s) messages too.

In this case, I think that the simplest way is that you squash branch headings-and-lists into dev, so you get a single commit, and then just push dev to this repository.

If it's too troublesome, just leave it as it is — also, I can always rewrite the history later, although it will then involve a hard reset of dev on your side.