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

Make CURRENT_LINE and CURRENT_COLUMN expansions one-based #8680

Conversation

sasumner
Copy link
Contributor

@sasumner sasumner commented Aug 7, 2020

Fixes #8679

@dail8859
Copy link
Contributor

dail8859 commented Aug 7, 2020

Couldn't this potentially break plugins that are expecting it to be zero based? Not saying it is correct, just that the behavior will change.

@sasumner
Copy link
Contributor Author

sasumner commented Aug 7, 2020

Couldn't this potentially break plugins that are expecting it to be zero based?

It could. Perhaps plugins was its primary use?
I can't see this at a user-level at all, as all other tools I've seen that deal with line and column numbers deal with "user" concepts of one-based numbers.

Should I back off and create $(CURRENT_LINE1) and $(CURRENT_COLUMN1) ?

@dail8859
Copy link
Contributor

dail8859 commented Aug 8, 2020

I guess that would be Don's call. That suggestion would ensure compatibility and would be my preference.

@sasumner
Copy link
Contributor Author

sasumner commented Aug 8, 2020

I guess that would be Don's call. That suggestion would ensure compatibility and would be my preference.

Let's just go with that, then.
No need to wait for Don on it.
I'll rework it...
Closing until rework is done.

@sasumner sasumner closed this Aug 8, 2020
@Yaron10
Copy link

Yaron10 commented Aug 8, 2020

Do plugin use $(CURRENT_LINE) or NPPM_GETCURRENTLINE?

If the latter is correct, how about changing wsprintf(expandedStr, TEXT("%d"), lineNumber); to wsprintf(expandedStr, TEXT("%d"), lineNumber + 1); instead of the current change?

(In RunDlg/RunDlg.cpp).

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.

CURRENT_LINE and CURRENT_COLUMN should be one-based not zero-based
3 participants