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

Add flag attribute to control case sensitivity of attribute resolution #509

Open
mojavelinux opened this issue Jul 17, 2013 · 16 comments
Open

Add flag attribute to control case sensitivity of attribute resolution #509

mojavelinux opened this issue Jul 17, 2013 · 16 comments
Assignees
Labels
Milestone

Comments

@mojavelinux
Copy link
Member

@mojavelinux mojavelinux commented Jul 17, 2013

By design, AsciiDoc treats all attribute names as lowercase, both when storing them and when resolving them. This is not always desirable. Introduce a document attribute flag that controls whether case sensitivity is honored in attribute names. The default should be compliant with AsciiDoc, which is to treat all attribute names as lowercase.

Example:

:name: 1
:NAME: 2
:case-sensitive-attrs:

{name} != {NAME}

Normally, the two attributes would resolve to the same value of 2.

@ghost ghost assigned mojavelinux Jul 17, 2013
@shahryareiv
Copy link
Contributor

@shahryareiv shahryareiv commented Jun 22, 2015

This should probably cover first-letter or full-word capitalization options also. I use attributes for highly used phrases (e.x. {hit} for health information technology), and acronyms.
In latex (through glossaries package), we can use \gls{hit}, \Gls{hit}, \GLS{hit} for different capitalization needs. In asciidoc I use {hit}, {hit-cap}, {hit-allcap} but probably {hit}, {Hit}, {HIT} should be a better solution (?)
Maybe related: the automatic expansion of an acronym in the first encounter is another issue. We do can it manually but it is error prone and also problematic for file inclusions. This is done in Latex through the same macro, ie \gls.

@mojavelinux
Copy link
Member Author

@mojavelinux mojavelinux commented Jul 4, 2015

Being able to turn off case insensitivity will allow you to define multiple variants like hit, Hit and HIT. This is another great justification for why we should have this control.

I think the expansion use case is best handled through a custom inline macro (e.g., abbr:HIT[]) instead of an attribute (or, in the future, a custom namespace for attributes like {abbr:HIT}). It could also be done with a Preprocessor. Extensions FTW!

@mojavelinux mojavelinux modified the milestones: v1.6.0, v1.7.0 Dec 21, 2015
@GeraldLoeffler
Copy link

@GeraldLoeffler GeraldLoeffler commented Aug 12, 2017

Has an expansion macro similar to the mentioned abbr been written?

@oddhack
Copy link

@oddhack oddhack commented Sep 9, 2017

I'd also find this useful - just got bitten by accidentally having two attributes in different case which I expected to be different.

@rockyallen
Copy link

@rockyallen rockyallen commented Oct 13, 2017

If this is to be the new preferred behaviour, the flag will have to be in every document forever. Would it be better to make it a breaking change, and only do the old (iilogical) behaviour in compatibility mode? I doubt that many people deliberately "used" the old style.

@rockyallen
Copy link

@rockyallen rockyallen commented Oct 13, 2017

Whatever the resolution, can you make it the same for counters?

@tajmone
Copy link

@tajmone tajmone commented Sep 26, 2018

Is this also going to affect tagged regions identifiers?

I'm currently building an AsciiDoc documentation from source generator that leverages tagged regions for building the extracted documentation:

When multiple regions with a same name are encountered in the parsed source, these are going to be merged into a single region in the final document. So, knowing if region tags should be treated case sensitively or not could affect the final results.

(I'm currently working on this regions merging feature, and while googling for case sensitiveness in region tag identifiers I've found this issue).

If you implement this change, could you also add a specific setting to control how include statements will treat region tags, allowing to control case sensitiveness of these independently of how attributes are being treated?

My appplication aside, this distinct setting would be useful when including portions of third party documents, as these might use different settings from the main document, and when selectively including regions from multiple sources being able to enable/disable case sensitiveness before an include directive would allow to set the include directive only, without affecting how attributes are treated in the current doc.

IMO allow case insensitiveness can introduce lots of complications when interacting with other documents, especially when including parts from other documents which have different settings. E.g., if my doc is set to case insensitiveness, and I include regions from another doc which assumes case sensitivness, the included regions will now follow the settings of my doc and could lead to unexpected results.

@mojavelinux
Copy link
Member Author

@mojavelinux mojavelinux commented Sep 27, 2018

Include tags are case sensitive. That is unlikely to change.

@mojavelinux
Copy link
Member Author

@mojavelinux mojavelinux commented Oct 9, 2018

If you implement this change, could you also add a specific setting to control how include statements will treat region tags, allowing to control case sensitiveness of these independently of how attributes are being treated?

I think tags should be case sensitive. I don't see any benefit to treating them as case insensitive.

Attributes are different since they have a history of being forced to lowercase, which can sometimes be counter-intuitive. To preserve compatibility, we are proposing to add a setting to disable that behavior (and thus treating them "as is").

@tajmone
Copy link

@tajmone tajmone commented Oct 9, 2018

So this new option will be switchable on/off inline? (i.e. not global)

Would it allow to include also documents which follow the old system (i.e. deliberately relying on case difference in attributes) by disabling it before an include directive and then re-enabling it after it.

If I've understood correctly, this option will make Asciidoctor lowercase internally the parsed attributes. If this is the case, and if Asciidoctor would still be internally referencing attributes in a case sensitive manner (i.e. the option just lowercases them at parse time to force the to match with each other) then mixing the old and new system shouldn't cause problems since any attributes parsed when the new option is off will retain their case.

@rockyallen

I doubt that many people deliberately "used" the old style.

I actually do use it in software documentation quite often, where I use custom macros for inline substitutions, and I want to distinguish between a class and its instance, for example.

@mojavelinux
Copy link
Member Author

@mojavelinux mojavelinux commented Oct 9, 2018

It will likely be a global option. We still need to decide if it's something that needs to be set in the document or can be set via the API.

If I've understood correctly, this option will make Asciidoctor lowercase internally the parsed attributes.

No. The option will make the attributes case sensitive. Currently, Asciidoctor (as did AsciiDoc Py before it) forced attribute names to lowercase. This can result in them not being matched in certain cases, such as when set from the API.

Frankly, the lowercasing of attributes was wrong. The processor should have just left them alone. So this issue is mostly trying to figure out a path to case sensitivity without wrecking compatibility.

@elextr
Copy link

@elextr elextr commented Oct 9, 2018

Frankly, the lowercasing of attributes was wrong. The processor should have just left them alone.

Correct.

So this issue is mostly trying to figure out a path to case sensitivity without wrecking compatibility.

It would have to be a command line setting would it not?

If its has to be set inside documents for them to be compatible then they are not compatible by definition 😁

@mojavelinux
Copy link
Member Author

@mojavelinux mojavelinux commented Oct 9, 2018

If its has to be set inside documents for them to be compatible then they are not compatible by definition

It depends. If we're turning case sensitivity on using the attribute, then only new documents that want it would have it. And so it would be compatible with older documents by default.

But if we do it the other way around...that's much more difficult.

We could also consider tying it to compat mode (though it would have been better to do that when compat mode was introduced).

@elextr
Copy link

@elextr elextr commented Oct 9, 2018

It depends. If we're turning case sensitivity on using the attribute, then only new documents that want it would have it. And so it would be compatible with older documents by default.

Ok, but annoying for future documents to have to set it in every document.

But if we do it the other way around...that's much more difficult.

Well yes, thats what I was thinking of.

We could also consider tying it to compat mode (though it would have been better to do that when compat mode was introduced).

No matter what other methods are used, it probably should be tied to compat mode anyway since its part of the Asciidoc Python behaviour.

Making incompatible changes compatible with both old and new docs is always an issue. Maybe it should be delayed until the next major version bump where such incompatibilities are expected.

@mojavelinux mojavelinux modified the milestones: v1.7.0, M3 Jan 9, 2019
@oliviercailloux
Copy link

@oliviercailloux oliviercailloux commented Feb 16, 2019

I am strongly in favor of Asciidoctor being opinionated on this issue. It should establish a standard, not just leave the choice to its users. So that when you edit a document that someone else has created, you can rely on the case-sensitive-behavior related to attributes being as usual and apply your usual Asciidoctor writing habits, not just because you thought about checking the header of the document, but because this is how everybody use it. The thing we certainly do not want is end up in a situation where half of the writers consider that attributes are case-sensitive and half do not.

So, I am in favor of waiting for an opportunity to introduce breaking changes and at that point, forcing (or encouraging very much) case-sensitive behavior, as there seems to be a consensus in this thread (to which I agree) that this is the sensible behavior.

Of course this is compatible with leaving the possibility to document authors to set up a compatibility mode to stick to the previous case-insensitive mode, if they absolutely need to (with the doc mentioning that it is discouraged for new documents). Hopefully the writer community will tend to not use this option except when absolutely required, and the standard will thus be followed for most use cases.

@mojavelinux
Copy link
Member Author

@mojavelinux mojavelinux commented Feb 19, 2019

I'm pretty strongly in favor of a) consistency and b) case-sensitivity. I don't really understand why the decision was made to drop the case in the first place (a decision that was introduced by AsciiDoc Python). Hardly anyone expects this, and it has lead to problems where the casing does not match.*

It's too late to introduce this change in 2.0.0, but it could certainly be done for a future major version, perhaps as early as 3.0.0. It will certainly be an important conversation in the spec process. (There may be tricks we can use to retain a certain degree of backwards compatibility).

For reference, Windows and macOS both have case-insensitive filesystems and it can create havoc. So there's some precedence for not ignoring case.

* If I can guess, the reason was to allow writing attributes entries in a "natural" style, like :Organization Name:, which then gets converted to organizationname. But this is really not necessary (and just more confusing).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.