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

Fix __FILE__ when filename contains quotes #566

Merged
merged 1 commit into from
Sep 10, 2020
Merged

Conversation

ZedKoS
Copy link
Contributor

@ZedKoS ZedKoS commented Sep 8, 2020

No description provided.

@AntonioND
Copy link
Member

The commit should describe the fix, not just refer to a GitHub issue. It's fine to have the issue reference, but you also need to describe what's going on in case the repository is being kept somewhere else. Something like:

Fix __FILE__ when name contains quotes

Escapes all quotes in __FILE__ so that it can be used in the code.

Fixes #546

@ZedKoS
Copy link
Contributor Author

ZedKoS commented Sep 8, 2020

Thanks for the feedback! I'll give at least a more appropriate name to the pull request.

@ZedKoS ZedKoS changed the title Fix issue #546 Fix __FILE__ when filename contains quotes Sep 8, 2020
@JL2210
Copy link
Contributor

JL2210 commented Sep 8, 2020

I wouldn't bother fixing this until the new lexer is merged; it'll just be overwritten if it is.

@ISSOtm
Copy link
Member

ISSOtm commented Sep 10, 2020

Considering that #557 may not be merged in a while, I'm gonna merge this anyways. It'll have to be added to #557 separately, as JL pointed out (sigh...), and will probably trigger a merge conflict anyways.

@ZedKoS Ideally, the commit message would also be changed (git commit --amend suffices when it's the last one, otherwise you should use git rebase -i); I'm gonna squash-merge the PR to avoid the problem anyhow.

@ISSOtm ISSOtm merged commit 34c2288 into gbdev:master Sep 10, 2020
@AntonioND
Copy link
Member

Yeah, I was talking about changing the commit message... But well, the squash merge kinda fixes the problem. Not ideal, but fair enough.

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.

4 participants