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

Inline fields not rendered correctly in reading view #2216

Closed
antoniak-piotr opened this issue Jan 20, 2024 · 14 comments · Fixed by #2377
Closed

Inline fields not rendered correctly in reading view #2216

antoniak-piotr opened this issue Jan 20, 2024 · 14 comments · Fixed by #2377
Labels
bug Something isn't working.

Comments

@antoniak-piotr
Copy link

antoniak-piotr commented Jan 20, 2024

What happened?

If multiple of the same inline fields are separated by a single empty line, they are rendered incorrectly. To see this behavior, you must enable the "Enable Inline Field Highlighting in Reading View" option. Simple example:

(note::Note 1)
(note::Note 2)
(note::Note 3)

Can someone confirm this bug?

DQL

No response

JS

No response

Dataview Version

0.5.64

Obsidian Version

1.5.3

OS

Windows

@antoniak-piotr antoniak-piotr added the bug Something isn't working. label Jan 20, 2024
@Elania
Copy link

Elania commented Jan 22, 2024

For me it happens only when every line is a task

- [ ] (note::Note 1)
- [ ] (note::Note 2)
- [ ] (note::Note 3)

then Note 1 is displayed three times in reading mode. In preview mode it's correct.
I have this in my daily template, so I switched to version 0.5.59 where it is not happening, but I didn't test every version in between.
Edit: See also this thread in the Obsidian forum: Repeated rendering in reading view of dataview inline fields for duplicated fields

@antoniak-piotr
Copy link
Author

antoniak-piotr commented Feb 2, 2024

I had some free time today so I played around a bit and found the solution. Unfortunately, I am not a DataView developer so I could experimenting and testing the solution only on the compiled, current version of the plugin (version 0.5.64) by making changes to the .obsidian\plugins\dataview\main.js file. Although diagnosing the cause took me a lot of time, the solution turned out to be very simple. To solve the problem I just modified one line of code. In the mentioned file, in function replaceInlineFields(ctx, init), I changed line 19309 from inlineFieldsFromText = extractInlineFields(text); to inlineFieldsFromText = extractInlineFields(text.split('\n')[_a.lineStart]);. I also found another, simpler, solution. To solve the problem it is sufficient to comment out the line 19358 (parsedValue = parsedValueFromText;). As I mentioned before I'm not a DataView developer so I don't know which solution is better, but if the latter, it would mean that the code of the replaceInlineFields function could be significantly slimmed down.
I would be grateful for information whether this solution also works for you.
If there is any DataView developer on the channel, I would be happy to know whether, and if so, why, it is necessary to consider inlineFieldsFromText and all the logic associated with it.
This comment is also published on Discord.

BTW. I think the solution should be implemented in src\api\views\inline-field.tsx, in function replaceInlineFields(ctx: MarkdownPostProcessorContext, ...). I suppose, but I am not sure of it, the problem can be solved by implementing something like this:

export async function replaceInlineFields(ctx: MarkdownPostProcessorContext, init: DataviewInit): Promise<void> {
    let inlineFields = extractInlineFields(init.container.innerHTML);
    if (inlineFields.length == 0) return;

    const info = ctx.getSectionInfo(init.container);
    const text = info?.text;
    let inlineFieldsFromText: InlineField[] | null = null;
    if (text) {
        inlineFieldsFromText = extractInlineFields(text.split('\n')[info.lineStart]);
    }

@holroy
Copy link
Contributor

holroy commented Feb 2, 2024

I tried your change, and at first it seemed to do the job. But after a little more testing, it fails again. There might be some caching issues as well. And I'm not sure if this fix tackles both the cases of round and square brackets around the inline fields.

I wonder if @RyotaUshio could look into this bug, as they was the one implementing the changes in this part of the code.

@holroy
Copy link
Contributor

holroy commented Feb 2, 2024

Attached to this comment are two markdown files showcasing how this is an issue for both variants of inline fields. The files also shows how it's just the main inline rendering which fails, and that the various query variants gets it correct. And it's just the reading mode which is affected.

t71031 repeated rendering round brackets.md
t71031 repeated rendering square brackets.md

Using Minimal theme the display of the top part of the round version of this test set displays as:
image

@antoniak-piotr
Copy link
Author

Yeah, definitelly there is something wrong. But, have you checked the second proposed solution? What I mean is to comment out line 19358 ("parsedValue = parsedValueFromText;). This solution renders both files properly (I hope), but as I mentioned earlier, it basically disables the entire inlineFieldsFromText` based part.
To be honest, I am really curious what is the purpose of that part.

@RyotaUshio
Copy link
Contributor

@holroy Thank you for letting me know, and I'm sorry if my code introduced this bug. I will look into this within a few days.

@phillipjohnston
Copy link

phillipjohnston commented Feb 12, 2024

Presumably related to this issue is code formatting breaks when :: is used, e.g., for std::vector which broke for me after updating Dataview.

- The C++ standard library frequently uses the Strategy pattern (often via "Policies" supplied via template parameters%%[[Policy-based design]]%%). Some examples include:
	- Selecting the `Allocator` strategy for a type (e.g., [`std::vector`](https://en.cppreference.com/w/cpp/container/vector))
	- Customizing the [`std::sort`](https://en.cppreference.com/w/cpp/algorithm/sort https://en.cppreference.com/w/cpp/algorithm/sort) algorithm by providing a comparison function.

This renders as:

Selecting the Allocator strategy for a type
Customizing the std. (https://en.cppreference.com/w/cpp/algorithm/sort https://en.cppreference.com/w/cpp/algorithm/sort) algorithm by providing a comparison function.

@holroy
Copy link
Contributor

holroy commented Feb 12, 2024

Another issue which I'm not sure if it ever has worked or not, but if do like the below it doesn't do inline queries correctly, it seems.

- [ ] (someKey:: hi there) and the value of it: `= this.someKey`

This could be a race condition issue, though, so I'm not quite sure if is supposed to be broken, or if it was broken with these changes, but I've never thought of it before lately, so....

@holroy
Copy link
Contributor

holroy commented Mar 26, 2024

We need to consolidate what works and what doesn't work, so could some of you please test the following markdown and report back when values are collated together or not?


forumUrl:: http://forum.obsidian.md/t//71031
githubUrl:: https://github.com/blacksmithgu/obsidian-dataview/issues/2216

## Unfixed?

### Multiple fields in list

Is the first line repeated in all four lines?

- [ ] (title:: "*The Lord Of The Rings Trilogy*"), (author:: "J.R.R. Tolkien")
- [ ] (title:: "*The Hitchhiker's Guide To The Galaxy*"), (author:: "Douglas Adams")
- [ ] (title:: "*Ender's Game*"), (author:: "Orson Scott Card")
- [ ] (title:: *The Dune Chronicles*), (author:: Frank Herbert)

### Inline fields with multiple values

Are these showing just one value in _live preview_ ?

[multiValueInt:: 1, 2]

[multiValueText:: "x", "y"]

## Fixed ?

Are the following OK in both _live preview_ and _reading mode_?

### Body

(body:: Body 1)
(body:: Body 2)
(body:: Body 3)

### Task

- [ ] (task:: Task 1)
- [ ] (task:: Task 2)
- [ ] (task:: Task 3)

### List multiple square brackets

- [duped:: a] and [not-duped:: a2]
- [duped:: b] and [duped-not:: b2]

### List multiple round brackets

- (duped:: a) and [not-duped:: a2]
- (duped:: b) and [duped-not:: b2]

When running within the test-vault with only dataview running on the code from 0.5.66, I only get errors related to the following two cases:

Multiple fields in list - Reading mode

image

Multiple values in inline fields - Live preview

image


Could you please verify that you see the same behaviour, and please also state which version of Obsidian and which operating system you're using. This test I've conducted on Win11 in Obsidian 1.5.11 (with 1.5.8 installer).

@rben01
Copy link
Contributor

rben01 commented Mar 26, 2024

@holroy I can confirm I'm seeing just the final value of multi-valued inline fields in live preview (#2281)

@holroy
Copy link
Contributor

holroy commented Mar 26, 2024

I've played around some more, and it seems like we might have an issue related to getting the right information back from the Obsidian API. Our code asks for the ctx.getSectionInfo(init.container) (where the init container is a given list item), and the information we get back pertains to the entire section of list items. I've posted some questions related to this in the Dev&API section on the Obsidian forums.

For the code to work we need it (or a similar method) to return the actual line number of the specific list item, and not the section which contains the list section. Otherwise it'll try to pull inline fields from the entire list (and not local to a given list item).

Earlier fixes, and why they fail

  • Using info.lineStart - This pulls the fields only from the first line of the list, even though we're having lists with multiple items
  • Using a variant of "text.slice(info.lineStart, info.lineEnd)` - Pulls fields from all the list item, not the one we're currently looking at
  • Using just init.container.innerText - This is better than the two above, but it doesn't carry the full raw text but a partially formatted text, so it fails to produce nice output
  • Commenting the parsedValue = ... - This do bring back the various fields, but it also removes extended formatting and parsing of the field values, which I reckon is why this code was added in the first place

@holroy
Copy link
Contributor

holroy commented Mar 27, 2024

I'm getting more and more confused on this bug. It disappears it seems if the Tasks plugin is enabled?!

The simples use case I've found so far is to:

  • Open the sandbox vault
  • Install Dataview and use the title/author task example from above
  • Switching to reading mode renders fields only from the first line
  • Installing Tasks and enabling it
  • Add a space to either line, to trigger a re-run
  • Switch to reading mode and the bug has disappeared
  • Disable Tasks plugin again, add a space to a line
  • Switch back to reading mode, and the bug re-appears

How are these connected!? What's different when Tasks are enabled? And can we duplicate this bug-fixing behavior?

Any ideas @blacksmithgu , @GottZ or @RyotaUshio ?

@GottZ
Copy link
Contributor

GottZ commented Mar 28, 2024

sadly I'm on vacation.. I have no clue but suspect that main.ts needs a proper cleanup.
it has a ton of fragmentation.
after the file context feature I currently implement, I'll likely move towards big chore tasks and improved overall testing

@holroy holroy mentioned this issue Apr 15, 2024
@holroy
Copy link
Contributor

holroy commented Apr 17, 2024

Related to my earlier attempts on this, and the realisation that the current logic can't be fixed since we don't have access to the exact line of a given list item (which is a key point of the current logic), should we revert the entirety of this change back to version 0.5.59?

Or possibly hide it behind some experimental toggle in the settings? This bug keeps hindering me in to give proper responses to guys at the forum, and it reduces the usability of Dataview to such an extent that I'm considering to either revert to version 0.5.59 altogether, or to branch of and start using just my own dedicated version of Dataview without this bug.

The original implementer of this change also hasn't responded, and most likely are not able to fix it either due to the poor access to the correct source code.

reply2za added a commit to reply2za/obsidian-dataview that referenced this issue Jun 18, 2024
blacksmithgu pushed a commit that referenced this issue Jun 20, 2024
* Fix inline rendering in reading view (#2216)

* fix: add math inline reading view rendering (#2084)

* refactor: update field semantic

* refactor: cleanup inline-field code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants