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

config: do not reset pos when braces found #3411

Merged
merged 1 commit into from
Jan 12, 2019

Conversation

RedSoxFan
Copy link
Member

Fixes #3410

When a brace is found, the config file should not seek back to before
the brace, otherwise the brace will be read multiple times.

When a brace is found, the config file should not seek back to before
the brace, otherwise the brace will be read multiple times.
@ianyfan
Copy link
Contributor

ianyfan commented Jan 12, 2019

I think this would be better placed in the else block of the if (strcmp(line, "{") == 0), so that it skips empty lines.

@RedSoxFan
Copy link
Member Author

The issue with that is the lines numbers for errors won't match the real file line number (note that this is already the case if the config file contains any continued lines since get_line_with_cont is assumed to be a single line.) Ideally, the numbers should be accurate to make finding and fixing the lines easier

@RedSoxFan
Copy link
Member Author

I think your PR actually fixed EOF not empty lines

@ianyfan ianyfan merged commit d256182 into swaywm:master Jan 12, 2019
@RedSoxFan RedSoxFan deleted the fix-brace-detect-seeking branch January 12, 2019 01:26
@ianyfan
Copy link
Contributor

ianyfan commented Jan 12, 2019

A fix for the continuation line problem would be good, too.

@ianyfan
Copy link
Contributor

ianyfan commented Jan 12, 2019

Thanks!

@ianyfan
Copy link
Contributor

ianyfan commented Jan 12, 2019

And an apology for completely forgetting how the code I wrote worked.

@RedSoxFan
Copy link
Member Author

A fix for the continuation line problem would be good, too.

Was going to include it in this one, but decided it would be better as a separate PR. Going to work on it now.

And an apology for completely forgetting how the code I wrote worked.

I mean it still looks a lot nicer than the code I wrote months ago that you replaced haha

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

Successfully merging this pull request may close these issues.

2 participants