platers / obsidian-linter

An Obsidian plugin that formats and styles your notes with a focus on configurability and extensibility.
https://platers.github.io/obsidian-linter/
MIT License
1.18k stars 79 forks source link

Bug: Alias key removed if "YAML Title Alias" enabled, alias is empty, and "Keep alias that matches the filename" disabled #1038

Open redactedscribe opened 6 months ago

redactedscribe commented 6 months ago

Describe the Bug

alias: key is removed by the steps below when it should be left blank instead.

How to Reproduce

Steps to reproduce the behavior:

  1. Ensure you have an empty YAML alias: key in the note.
  2. Enable "YAML Title Alias"'s first option to get alias from H1 or filename.
  3. Ensure "Keep alias that matches the filename" option is disabled.
  4. Lint the note.
  5. alias: key is removed from the note.

Alias options which may be related: When you search "alias" in Linter's settings, I have all visible toggles enabled, except for "Keep alias that matches the filename" (at bottom).

Expected Behavior

The alias: key should not be removed in this scenario, but simply left as-is.

Expected output if applicable:

---
alias: []
---

Device

Additional Context

Ideally both of the above double-lints would instead just require a single lint. There are probably other double-lint issues relating to these title alias options.

Thanks.

pjkaufman commented 6 months ago

Hey @redactedscribe , I will need to take a closer look at the code and see what is going here. If I recall correctly the rule for YAML Title Alias will remove the alias key if after it does it operations it finds that there are no entries in that key. That is especially true when saying not to retain the note title as an alias.

For clarity's sake, do you expect alias to be retained in the following scenarios or just the first one?

As for the double lint:

  • Enabling the "Keep alias that matches the filename" option after step 5 above and re-adding alias: [] to the note, and then linting, two lints are required because the first does alias: [my note] and the second converts it to a multiline array (as per my linter settings).

This sounds like a bug since it should be using the specified Linter settings. I will need to test this scenario and see what is going on.

  • Similarly, the "Sorts YAML array values based on the specified sort order." option doesn't seems to run after these title alias operations because a second lint is required to then sort the alias array.

This is caused by sort happening in alphabetical order based on the rule id/name under the hood. This can be fixed by forcing the rule to run as a special case after all of the other rules.

redactedscribe commented 6 months ago
  • The alias key is initially blank and after lint the alias key is blank for YAML Title Alias

    • Currently results in a removal of the key from my understanding.

It removes the blank alias key when "Keep alias that matches the filename" option is disabled when it shouldn't remove the key; there is nothing to do, why remove the key... (my other blank keys remain present).

  • The alias key is not initially blank, but after lint the alias key is blank for YAML Title Alias

    • I believe this removes the key as well. This would be a scenario where you do not want the file name as an alias, but it is the only current alias.

No, if alias isn't blank with the same Linter config, the alias data isn't modified after listing. The issue appears limited to the "YAML Title Alias"'s first option to get alias from H1 or filename which is removing the blank alias key when it should leave it present.

pjkaufman commented 6 months ago

So I think we are partially talking past each other which is probably because I did not include examples. But there are two secnarios where after running YAML Title Alias you end up with no values for the alias key.

Scenario 1: You start out with a blank key and end with a blank key. The reason the key gets removed from my understanding is because it clutters up the YAML section. You could argue now that properties gets rid of the need for that, but this predates that by a ways.

This scenario is one I think we can update to make it so that the alias section is not removed because the rule did not change any values. Or for backwards compatibility, we could add an option that lets you specify whether or not to remove the alias section when blank which would default to true since that is the default behavior.

Scenario 2: You start out with just the filename or h1 as the only alias and have the option to not keep that alias checked. In this case, the alias key is removed to declutter the YAML because it removed a value.

I think that the simplest solution would be to expose an option that lets the user decide if they want the alias key removed when empty for this rule and it would be respected by both scenarios.

What do you think?

redactedscribe commented 6 months ago

As a test, I thought adding aliases to the "Force key values to be [single|multi]-line arrays" option may solve the issue but it does not. Neither did adding aliases: to option "Insert YAML attributes".

This scenario is one I think we can update to make it so that the alias section is not removed because the rule did not change any values. Or for backwards compatibility, we could add an option that lets you specify whether or not to remove the alias section when blank which would default to true since that is the default behavior.

In this case, it seems to me that an option should be provided, but should require manually changing if you want the current behaviour to change. On fresh configs, I'd argue that not removing the YAML key should be the default, simply because all other keys, single- or multi-line aren't removed if blank on linting.

I think that the simplest solution would be to expose an option that lets the user decide if they want the alias key removed when empty for this rule and it would be respected by both scenarios.

This sounds sane, however it could be a broader feature:

I don't think there is a "Remove empty keys" option yet, but I do see a "Remove YAML Keys" option. At first I thought: If "Remove YAML Keys" had a "Remove empty keys only" sub-option, keys listed, such as aliases, if empty, could get removed through this rule, but I think it'd be better if it were an option unto itself, i.e. not coupled. This way, keys could be removed, regardless of them being empty or not, via "Remove YAML Keys", and a separate "Remove empty keys" toggle rule to simply ensure any empty keys always get removed.

The reason for a more broad rule is that I don't see why aliases should be treated any more special than any other key. Yes, aliases is one of the more common / standard keys, but does it really need special treatment?

pjkaufman commented 4 months ago

This seems to have been an issue in the past: #820 .

Sorry about the delay in responding. I don't know that I added the logic that removes the empty alias key. I can only say that I believe that when it was added the idea was that we want to keep the YAML as decluttered as possible. So when there is no entry in the aliases key, it gets removed. There can be a feature to expose the ability to control this functionality if that is where this is headed.