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

Add configuration for overriding keyboard layout #526

Merged
merged 2 commits into from May 23, 2019

Conversation

yous
Copy link
Member

@yous yous commented May 21, 2019

Screenshot 2019-05-21 21 04 25

@youknowone
Copy link
Member

#490

@yous yous marked this pull request as ready for review May 22, 2019 05:59
return unmanaged.takeRetainedValue() as? [TISInputSource]
}

func property(forKey key: CFString) -> Any? {
Copy link
Member

Choose a reason for hiding this comment

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

다른 함수의 관례를 따르면 String이나 NSString을 받고 함수 안에서 변환하는게 좋겠습니다

Copy link
Member Author

Choose a reason for hiding this comment

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

kTISProperty... 상수들이 CFString이라 이렇게 뒀는데 String으로 바꾸는 게 나을까요?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

이런 식으로 바꿔봤습니다. 74dbd55

@IBOutlet var romanModeByEscapeKeyButton: NSButton!
@IBOutlet var hangulAutoReorderButton: NSButton!
@IBOutlet var hangulNonChoseongCombinationButton: NSButton!
@IBOutlet var hangulForceStrictCombinationRuleButton: NSButton!

var configuration = Configuration()
var inputSources: [(identifier: String, localizedName: String)] = []
Copy link
Member

Choose a reason for hiding this comment

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

lazy var inputSources: [(identifier: String, localizedName: String)] = {
   return loadInputSources()
}()

아니면 그냥 loadInputSources()를 지우고 여기로 옮겨도 좋겠습니다

Copy link
Member Author

Choose a reason for hiding this comment

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

loadInputSources() 안에서 overridingKeyboardNameComboBox.reloadData()를 호출하는데 closure 안에 있으면 inputSources가 변경되기 전에 호출될 것 같네요. 좋은 방법이 있을까요?

Copy link
Member

Choose a reason for hiding this comment

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

overridingKeyboardNameComboBox.reloadData()를 viewDidLoad에 넣으면 어떨까요?
loadInputSources함수를 사이드이펙트 다 없애고 inputSources로 옮기면 reload할때 자연스럽게 inputSources를 참조하면서 목록을 불러올것 같습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

잘 동작하는 것 같습니다. 64c83dd

if sourceUS != nil && !sources.contains { $0.0 == sourceUS.0 && $0.1 == sourceUS.1 } {
sources.append(sourceUS)
}
sources.sort { $0.localizedName < $1.localizedName }
Copy link
Member

Choose a reason for hiding this comment

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

US, 활성자판, 비활성자판 순으로 정렬할거면 어차피 전체 자판에서 US가 포함될거고 정렬함수만 잘 만들면 되니까 sourceUS 없이 US layout identifier만 잘 들고 정렬에 쓰면 될것 같아요

Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -912,6 +913,7 @@
developmentRegion = English;
Copy link
Member

Choose a reason for hiding this comment

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

이걸 en으로 바꾸면 밑에서 knownRegions에 English가 추가 안될것 같네요

Copy link
Member Author

Choose a reason for hiding this comment

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

libhangul-objc도 비슷하게 수정해야 하네요. deprecated language 경고를 수정하면 되는 듯합니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

복병이 여기저기 숨어있었네요 감사합니다


loadInputSources()
for index in 0 ..< inputSources.count {
if configuration.overridingKeyboardName == inputSources[index].identifier {
Copy link
Member

Choose a reason for hiding this comment

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

if let selectedSource = inputSources.first(where: {$0.identifier == configuration.overridingKeyboardName}) {
   ...
}

Copy link
Member Author

Choose a reason for hiding this comment

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

firstIndex로 하니 깔끔해졌네요. e22476c

}

var enabled: Bool {
get {
Copy link
Member

Choose a reason for hiding this comment

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

get {} 없이 코드 블럭이 바로 나오면 getter가 됩니다. swiftformat을 한번 돌리면 자동으로 됩니다. (brew로 받을 수 있습니다)

Copy link
Member Author

Choose a reason for hiding this comment

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

} else if $1.identifier == usIdentifier {
return false
}
if $0.enabled && !$1.enabled {
Copy link
Member

Choose a reason for hiding this comment

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

if $0.enabled != $1.enabled {
    return $0.enabled
}

Copy link
Member Author

Choose a reason for hiding this comment

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

@youknowone youknowone merged commit 0104cf6 into gureum:master May 23, 2019
@yous yous deleted the override-layout branch May 23, 2019 07:54
@youknowone
Copy link
Member

#103 도 깔끔하게 해결될듯 합니다

@youknowone
Copy link
Member

#51

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.

None yet

2 participants