Gibberlings3 / iesdp

Jekyllized version of IESDP with imported history
https://gibberlings3.github.io/iesdp/
19 stars 22 forks source link

Opcode #135 / opcode #335 #130

Closed 4Luke4 closed 2 years ago

4Luke4 commented 2 years ago

Added some rudimentary info about how to bypass the bug affecting these two opcodes.

lynxlynxlynx commented 2 years ago

Please use the relurl function as discussed, so this will work properly also for @burner1024's projects.

4Luke4 commented 2 years ago

Please use the relurl function as discussed, so this will work properly also for @burner1024's projects.

Isn't that only for yml files...? I edited only standard html files this time...

burner1024 commented 2 years ago

To work in opcodes, relurl.html should also be included in op-list.html.

4Luke4 commented 2 years ago

To work in opcodes, relurl.html should also be included in op-list.html.

Is everything OK, now?

4Luke4 commented 2 years ago

Include before the for loop, otherwise it'll be included hundreds of times

Fixed.

lynxlynxlynx commented 2 years ago

It's fine if @burner1024 doesn't care, otherwise just adding the include doesn't change anything.

burner1024 commented 2 years ago

At the moment opcode descriptions are not being imported. But they likely will be sooner or later.

I think it makes sense to have a consistent policy for links, applying relurl everywhere. Anyway, it's not something that's going to be done quickly, IESDP is too vast. Probably just fix them along the way while working on something else, like this pull?

lynxlynxlynx commented 2 years ago

I agree, that's why I proposed to use it. @4Luke4 you can search for relurl to see how it's used as a liquid filter.

4Luke4 commented 2 years ago

@4Luke4 you can search for relurl to see how it's used as a liquid filter.

Will do. Feel free to merge then...

lynxlynxlynx commented 2 years ago

Sorry, but the consensus above was to fix this now, not later, so we can get farther incrementally.

4Luke4 commented 2 years ago

Sorry, but the consensus above was to fix this now, not later, so we can get farther incrementally.

@burner1024 OK, so what exactly...? Should I add {% include relurl.html %} to some other file...?

burner1024 commented 2 years ago

I think what @lynxlynxlynx means is that urls in this pull should be changed to follow the new scheme (with relurl filter.)

4Luke4 commented 2 years ago

I think what @lynxlynxlynx means is that urls in this pull should be changed to follow the new scheme (with relurl filter.)

Could you please check if everything is fine now...?

burner1024 commented 2 years ago
<a name="class" href="{{ '#op337' | prepend: relurl }}">opcode #337</a>

"class" is just an example, for when you do want to add a name to the link, and markdown doesn't support it, and then we use html intead. You don't need to add it to every link. And there's no need to add relurl to anchors that lead to the same page. They are relative anyway:

<a href="#op337">opcode #337</a>

<a name="class" href="{{ '../file_formats/ie_formats/eff_v2.htm' | prepend: relurl }}">EFF</a>

All urls should start from root of the site. No ../ anywhere:

<a href="{{ '/file_formats/ie_formats/eff_v2.htm' | prepend: relurl }}">EFF</a>
4Luke4 commented 2 years ago

All urls should start from root of the site. No ../ anywhere:

This leads to an error

`/file_formats/ie_formats/eff_v2.htm' not found.
burner1024 commented 2 years ago

Where? Push to your branch, point the exact link, I'll check.

lynxlynxlynx commented 2 years ago

Maybe a missing include?

4Luke4 commented 2 years ago

Maybe a missing include?

Where exactly...?

lynxlynxlynx commented 2 years ago

Is there an {% include relurl.html %} somewhere before the call in that file or where it gets included itself?

4Luke4 commented 2 years ago

Where? Push to your branch, point the exact link, I'll check.

Done.

The url {{ '/file_formats/ie_formats/eff_v2.htm' | prepend: relurl }} leads to the aforementioned error...

4Luke4 commented 2 years ago

Is there an {% include relurl.html %} somewhere before the call in that file or where it gets included itself?

There is one in _includes/op-list.html...

burner1024 commented 2 years ago

Yes, it's not working. Apparently collection content gets evaluated before the inclusion.

4Luke4 commented 2 years ago

Yes, it's not working. Apparently collection content gets evaluated before the inclusion.

Ok, so what now...?

burner1024 commented 2 years ago

I quickly tried liquify/markdownify filters, didn't help, but maybe @lynxlynxlynx has a better idea.

If eventually opcodes are going to be moved to _data, can just leave them as is for now, and fix later.

lynxlynxlynx commented 2 years ago

Put the include in the opcode file itself, then it should work.

burner1024 commented 2 years ago

Right, except there's hundreds of them, not a very clean solution.

lynxlynxlynx commented 2 years ago

Most don't link anywhere, so it's fine in the interim. Easy to automatically remove when a better solution is available too.

4Luke4 commented 2 years ago

Put the include in the opcode file itself, then it should work.

Done.

@burner1024 Do you want me to remove <code> tags when using <a href>...?

lynxlynxlynx commented 2 years ago

op335 still needs updating

4Luke4 commented 2 years ago

op335 still needs updating

Done.

lynxlynxlynx commented 2 years ago

Thanks and thanks for the patience!

burner1024 commented 2 years ago

@4Luke4

@lynxlynxlynx

  1. Should I send a pull adding relurl to all opcodes?
  2. To be clear, is moving opcodes to data in the plan or not? If yes, I'll probably eventually send a pull for that as well, when I find the time to make the conversion. Shouldn't be too hard, considering it's pretty much structured, mostly it'll be html > markdown thing, although that could bring a few surprises.
4Luke4 commented 2 years ago
  • Whereas [`x`](y) turns into a proper link.

True, but then x will no longer be highlighted in blue... As a result, I think I'll remove the <code> tags...

burner1024 commented 2 years ago

It should, though.

4Luke4 commented 2 years ago

It should, though.

Yeah, me dumb: I had forgotten to update my fork to include your latest changes to _sass/_general.scss...

lynxlynxlynx commented 2 years ago

@burner1024

  1. I don't think that's needed, we can do it as we start using more and more relurl.
  2. You can go ahead; I see we already discussed this a bit in #84 (concluding the notes should be left inline).