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(developer): various project fixes 🦕 #10151

Merged
merged 6 commits into from
Dec 12, 2023

Conversation

mcdurdin
Copy link
Member

@mcdurdin mcdurdin commented Dec 6, 2023

A single PR for 4 separate fixes to Keyman Developer, all relating to Projects. The fixes are separated by commit but were small enough and isolated enough that I didn't think I needed to split into separate PRs (yeah, yeah, I know).

@keymanapp-test-bot skip

I have already tested these locally and do not think they warrant separate user testing at this point.

Fixes #10144.

Also adds .keyboard_info, for now, although it is removed in 17.0, as
there will be numerous projects which still have a .keyboard_info and
it is handy to be able to load it and view its contents while upgrading.
Fixes #10145.

Turns out we were clobbering out internal data because Delphi strings
are copy-on-write. At the same time, moved from using the Hint property
to just referencing the source filename in the array, as that is cleaner
anyway.
Fixes #10146.

Some developers may wish to stick with kpj-1.0 for now, and the prompt
to upgrade would annoying because it cannot be hidden. This resolves
that.

Note that I haven't at this point renamed UrlRenderer.pas, although it
may be worth considering in the future.
Fixes #10148.

We now force the SourcePath and BuildPath project properties to be a
direct subfolder of the project folder, in order to avoid issues with
paths in the future. This is only applied to v2.0 projects, and only
enforced via the UI at this point.
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Dec 6, 2023

@@ -29,7 +29,7 @@ inherited frmOptions: TfrmOptions
Top = 185
Width = 125
Height = 25
Caption = '&Proxy Settings...'
Caption = 'Prox&y Settings...'
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: I fixed up overlapping shortcut keys in this dialog pane.

Comment on lines +234 to 243
var
buffer: array[0..MAX_PATH] of char;
begin
Result := Files[Index];
StrPCopy(buffer, Files[Index]);

if PathCompactPath(0, PWideChar(Result), GetSystemMetrics(SM_CXSCREEN) div 3) then // I4697
Result := string(PChar(Result)) // This removes the terminating nul
if PathCompactPath(0, buffer, GetSystemMetrics(SM_CXSCREEN) div 3) then // I4697
Result := buffer // This removes the terminating nul
else
Result := ExtractFileName(Files[Index]);
end;
Copy link
Member Author

Choose a reason for hiding this comment

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

This was badly buggy previously -- PathCompactPath would overwrite Result (note that Result was not guaranteed to be MAX_PATH which is technically a violation of the API contract but was a secondary issue), but as Delphi strings are copy-on-write, it ended up overwriting Files[Index] as well!

@mcdurdin mcdurdin changed the title fix(developer): various project fixes fix(developer): various project fixes 🦕 Dec 6, 2023
Copy link
Contributor

@darcywong00 darcywong00 left a comment

Choose a reason for hiding this comment

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

lgtm

@mcdurdin mcdurdin modified the milestones: A17S27, A17S28 Dec 8, 2023
@mcdurdin mcdurdin merged commit 1f06224 into master Dec 12, 2023
8 checks passed
@mcdurdin mcdurdin deleted the fix/developer/10144-10145-10146-10148 branch December 12, 2023 21:45
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 17.0.229-alpha

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