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 path placeholders for threads #1192

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

CanePlayz
Copy link
Contributor

@CanePlayz CanePlayz commented Jan 24, 2024

Closes #1123

This pull request introduces three new path parameters (currently called %x, %y, and %z because that was what came to my mind, please let me know of alternatives that make more sense). If the exported channel is a thread, these parameters are resolved to either the ID, the position, or the name of the thread's parent channel.

If the channel isn't a thread, the entire structure where one of these parameters is included (//...%y...//) is removed. The PR also modifies the logic of the category placeholders so that these correctly resolve to the category if the exported channel is a thread and not to its parent channel.

Previously, it wasn't possible to include both the category and the parent channel in the path of an exported thread.

I've also added .vscode/ to the .gitignore file since I prefer working with VS Code, but feel free to remove it if it shouldn't be there for some reason.

Also, please let me know if there's a better way to do it and to improve my coding style, I have almost no experience with C#.

In case this approach will be accepted, I'll also add the documentation for it.

@CanePlayz CanePlayz changed the title Add Path Placeholders for Threads Add path placeholders for threads Jan 24, 2024
@CanePlayz
Copy link
Contributor Author

On this topic, is there any pattern in the current naming scheme of path placeholders or in the way they are structured in the code (these blocks of two parameters)?

@Tyrrrz
Copy link
Owner

Tyrrrz commented Nov 6, 2024

@CanePlayz sorry for dragging this for so long.

Some version of the placeholder rework will be included in 3.x as part of #1271. The original system didn't expect channels to have parents or even have a deep hierarchy (which is possible as categories are considered channels in Discord API and in DCE).

I think ultimately I'm leaning towards two generic placeholders:

  • %h: replaced by parent/parent/child which can result in nested directories.
  • %H: replaced by parent - parent - child (or some other separator) which does not result in nested directories, but can still be used to incorporate the hierarchy in the filename.

I think it should essentially solve #1123, but without tying ourselves too much to the system we currently have.

In fact, as I'm writing this, it's probably possible to implement this before 3.x, just keeping both options available side by side.

I think this solution should be pretty similar to what you suggested here, except generic enough to encompass both categories and threads. I don't know if there's a use case to preserve hierarchy just for thread/channel relationship, but not include categories. It's possible to add another placeholder for that, but I think it's better to keep things future-proof and not rely too much on what's possible or impossible with Discord channels today, as we already made that mistake before.

@CanePlayz
Copy link
Contributor Author

Hmm, I see your point of keeping it rather simple, but it would definitely take away some options. I currently use a structure of Cat Number - Cat / Channel Number - Channel / Thread Number - Thread which I couldn't implement anymore with your suggestion.

@Tyrrrz
Copy link
Owner

Tyrrrz commented Nov 7, 2024

Hmm, I see your point of keeping it rather simple, but it would definitely take away some options. I currently use a structure of Cat Number - Cat / Channel Number - Channel / Thread Number - Thread which I couldn't implement anymore with your suggestion.

Yeah, that would not be possible then.

The other alternatives are either parameterized placeholders (something similar to #1170) or placeholders tailored specifically for threads (as implemented here). The first I don't like due to complexity, the second I don't like due to the risk of pigeonholing ourselves out of future changes on Discord's side. My solution, in turn, lacks flexibility, but is both simple and future-proof. I don't currently see a solution that has all three traits at the same time.

@CanePlayz
Copy link
Contributor Author

Although I don't necessarily see the risk of us being tied too closely to Discord's future update swith this solution and mainly see the flexibility as a benefit, I can understand the worries behind it. And after all, it's your application, so it's ultimately up to you. I can still use my fork for my own needs in case out-of-the-box DCE doesn't 100% fit my needs.

@Tyrrrz
Copy link
Owner

Tyrrrz commented Nov 7, 2024

@CanePlayz are you using your fork currently to generate the paths as in the example above?

@CanePlayz
Copy link
Contributor Author

Yup, I am

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.

Make template tokens work properly for export paths of threads
2 participants