Skip to content

fix: filter newline#2526

Open
valoq wants to merge 2 commits intogokcehan:masterfrom
valoq:protocol
Open

fix: filter newline#2526
valoq wants to merge 2 commits intogokcehan:masterfrom
valoq:protocol

Conversation

@valoq
Copy link
Copy Markdown
Contributor

@valoq valoq commented Apr 14, 2026

we are using newline as a delimiter, so we cant have this in filenames

Copy link
Copy Markdown
Collaborator

@CatsDeservePets CatsDeservePets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also filter these out before writing marks and tags.

@valoq
Copy link
Copy Markdown
Contributor Author

valoq commented Apr 14, 2026

Please also filter these out before writing marks and tags.

Thanks for catching that.
I went through it again and found a bunch of other functions that have the same issue.

I was contemplating this for a while, but I also added an error log instead of just skipping the file, so at least its in the logs. Ui error display did not seem appropriate for these functiosn though.
In case of listFilesInCurrDir the err log is skipped because that would trigger by just visiting a directory, but maybe we should it there too?

I would like some feedback on the approach for now.

@valoq valoq requested a review from CatsDeservePets April 17, 2026 08:20
@CatsDeservePets
Copy link
Copy Markdown
Collaborator

In case of listFilesInCurrDir the err log is skipped because that would trigger by just visiting a directory, but maybe we should it there too?

Better not, this would mess up my logs due to macOS having Icon\r system files.

I would like some feedback on the approach for now.

Sorry for not replying earlier. I had some stressful days. I could not come up with a 'better' solution here, so I think this is fine.

@valoq
Copy link
Copy Markdown
Contributor Author

valoq commented Apr 27, 2026

Sorry for not replying earlier. I had some stressful days. I could not come up with a 'better' solution here, so I think this is fine.

Take as much time as you need. I already feel bad for all the PRs anyway

@CatsDeservePets
Copy link
Copy Markdown
Collaborator

If you have nothing to add, I'd merge it.

@valoq
Copy link
Copy Markdown
Contributor Author

valoq commented Apr 27, 2026

Nothing more to add, ready to be merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants