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

Fix syntax coloring problem by using character ref for apostrophe. #1170

Merged
merged 2 commits into from
Oct 3, 2019

Conversation

skynavga
Copy link
Collaborator

@skynavga skynavga commented Oct 2, 2019

Trivial editorial change only.

@skynavga skynavga added this to the 2ED-FPWD milestone Oct 2, 2019
@skynavga skynavga self-assigned this Oct 2, 2019
@nigelmegitt
Copy link
Contributor

I don't see a syntax coloring problem - is this to help you work in your editing environment @skynavga ? (just wondering if a better solution isn't to use an IDE that doesn't get confused by apostrophes...)

@skynavga
Copy link
Collaborator Author

skynavga commented Oct 2, 2019

@nigelmegitt yes, emacs; i'm not open to changing editors, as I've been using this one for 45 years

@nigelmegitt
Copy link
Contributor

Hmm, sorry to be a pain on this one but I just opened up the spec XML in a vanilla emacs installation and there's no problem, which suggests it is a local emacs configuration issue:

image

On the grand scale of things this change is extremely minor; I'm worried that adding a new style rule for editing TTML2 that requires that the entity reference is used instead of apostrophes is going to be hard to remember and will make spec editing harder for all the Editors, and it doesn't seem like a great precedent to work around a local IDE issue with a global spec edit.

@nigelmegitt
Copy link
Contributor

OK I've expressed a concern about the proposal with an understanding that the issue is minor, and received a disproportionate threat in response. This isn't good. I'm going to back off this now; perhaps another Editor has a view and could approve if they're happy with this? I won't block it. pinging @cconcolato @palemieux

For reference I'm using emacs 22.1.1 that appears to have come pre-installed on my Mac; certainly I have no recollection of installing it, nor any evidence that I did so.

@skynavga
Copy link
Collaborator Author

skynavga commented Oct 3, 2019

@nigelmegitt I use emacs 23.6 from macports rather than the apple default build (22.1). The newer version has a much changed syntax parsing configuration for xml and nxml modes. I use the nxml mode. This problem has been present for a while, which I have dealt with in the past by inserting <!-- " --> and <!-- ' --> comments. But this workaround has become untenable, and the only reasonable alternative is to use &apos; and &quot; as needed to ensure the syntax parser is happy.

When I am no longer editor, then someone can change these back to the non-entity characters if they wish, but, in the mean time, I need this change to be able to do my day to day editing. Furthermore, given the length of the ttml2.xml source file, it has become progressively more difficult to track down syntax problems that crop up from time to time. And to do this, I need my editor (emacs 23.6 with nxml mode) to deal with this situation.

What I find most disconcerting is that you should decide to withhold your approval for a trivial editorial change that should be routine by nature and which requires no comment at all. You are not the one doing day to day editing, and I am the only active editor. Would you prefer to delay my work and delay going to FPWD simply because you find my customary editing practice different than your own?

We have already invested far more time and energy into this routine editor's PR than is warranted. Is that what you would insist we do to make progress?

Copy link
Contributor

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving as per meeting 2019-10-03.

@css-meeting-bot
Copy link
Member

The Timed Text Working Group just discussed Fix syntax coloring problem by using character ref for apostrophe. ttml2#1170, and agreed to the following:

  • RESOLUTION: Accept this pull request
The full IRC log of that discussion <nigel> Topic: Fix syntax coloring problem by using character ref for apostrophe. ttml2#1170
<nigel> github: https://github.com//pull/1170
<nigel> Nigel: Seeking a view from TTML editors?
<nigel> Pierre: It's a pain to do this. There are 250 changes.
<nigel> Cyril: It's difficult, I agree with Nigel that it will not be easy for other Editors to remember it and they might forget to do it.
<nigel> .. But I also think that Glenn is doing all the editing these days and we heavily rely on him so if it hinders him
<nigel> .. from doing his job we should accept the change. What he said is also correct that future editors can revert the change.
<glenn> Without this change, I cannot do any further edits.
<nigel> Cyril: On the editorial side I would approve the request.
<nigel> .. The burden will be on Glenn to maintain it if other Editors contribute and use ' instead of &apos;
<glenn> And it is a trivial change using M-x query replace.
<nigel> PROPOSAL: Accept this pull request
<glenn> Sure, that's reasonable.
<nigel> Nigel: Any objections?
<nigel> Pierre: I don't object to this change but the characterisation on the pull request is wrong - it is not trivial.
<nigel> RESOLUTION: Accept this pull request
<nigel> Nigel: I will press the Approve button.
<glenn> Thanks
<nigel> .. Done

@skynavga skynavga merged commit 7784b1e into master Oct 3, 2019
@skynavga skynavga deleted the ga/fix-apos branch October 3, 2019 16:16
@skynavga skynavga removed their assignment Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants