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

Handle filenames with spaces #44

Merged
merged 2 commits into from
Jun 4, 2017

Conversation

alexander-bauer
Copy link
Contributor

  • This breaks all tests, because expected filenames no longer match the ones that are actually written. @pkgw I would appreciate your input on how to fix this, or what should be done about it.
  • There was originally a test added in this PR for the behavior, but it failed even after the change was made (and not for the quoting reason.) I don't know what was wrong with it, but a better test should be written instead.

For my notes while developing this, see my comments in #36.

- This breaks all tests, because expected filenames no longer match the
  ones that are actually written
- Spaces are handled correctly in filenames, so this fixes tectonic-typesetting#36
- This fixes the tests again
- Spaces are still handled correctly, to fix tectonic-typesetting#36
@alexander-bauer
Copy link
Contributor Author

The tests are no longer broken, because filenames are now quoted only if necessary. New test cases will have to be written with this in mind.

@pkgw
Copy link
Collaborator

pkgw commented Jun 2, 2017

Thanks for tackling this problem. I have to say that the current version of the solution feels a bit fragile to me; what happens if the user passes in a filename that itself contains quotation marks? And so on.

Can you point me to the code which ends up reading and parsing the quoted string? I am wondering if we can give the relevant function a special flag that tells it basically "just use this sequence of bytes as the input file name, no questions asked", and then activate that flag for the toplevel input file. I think that would be more robust against the other kinds of funky filenames that might be passed in.

@alexander-bauer
Copy link
Contributor Author

@pkgw The file name scanning happens in scan_file_name(void), in tectonic/xetex0.c:10660-10680. This function operates on a bunch of variables that I didn't find the declarations of, so I was hesitant to mess with it.

If we're going to change things at this level, it looks like we should look at more_name(UTF16_code) in tectonic/xetex0.c:10425-10455. I suspect we could change it to simply terminate if the byte passed in is null. I'd be a little worried about side effects, because I have no idea what else uses that piece of code.

@alexander-bauer
Copy link
Contributor Author

@pkgw In fact, if we modify it to stop on null bytes (I think modification is necessary, but perhaps I'm not reading carefully enough), then we can probably just turn off the flag stop_at_space.

@alexander-bauer
Copy link
Contributor Author

I've experimented a little with null-terminating the string and turning off that flag, and I haven't been able to get our code to behave without breaking something else down the line. Maybe it would be acceptable to make the quoting code a bit smarter: any unquoted sequence of spaces is itself quoted, and otherwise the string is left alone.

That is certainly more fragile than I'd like, but there's a lot to untangle in order to make it better, I think.

@pkgw pkgw merged commit c5dab85 into tectonic-typesetting:master Jun 4, 2017
@pkgw
Copy link
Collaborator

pkgw commented Jun 4, 2017

Thanks for your work on this. I would like to credit you in the release notes for the next release; would you prefer that I identify you as Alexander Bauer, @alexander-bauer, or some other way?

I've been thinking about the intersection of this issue and the other issues relating to input file paths. I think I see a more robust solution, and I hope to implement it soon.

@alexander-bauer
Copy link
Contributor Author

@pkgw Thanks very much for your recognition! I'd love to be credited as Alexander Bauer, and if it's in a context where GitHub user-linking will work, an @alexander-bauer link to my profile. Otherwise, just the name would be fantastic!

@alexander-bauer alexander-bauer deleted the issue-36 branch June 4, 2017 16:43
Mrmaxmeier pushed a commit to Mrmaxmeier/tectonic that referenced this pull request Oct 1, 2019
restore enums, datetime::LocalTime, use ttstub
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