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

Preferences dialog #130

Merged
merged 32 commits into from
Oct 18, 2019
Merged

Conversation

jecaro
Copy link
Contributor

@jecaro jecaro commented Oct 14, 2019

Here is my take for a basic preference dialog #82

I did not spend much time trying to make the dialog looks good. I've focused on making the feature works.

I was confused to see the settings related to the current font in tmStateFontDesc and in tmStateConfig/options/fontConfig. It is not clear to me why it needs to be that way.

Anyway, so when the user validates the dialog, I decided to update the information at both places. Note that it is not the case when the font size is changed using the menu. In this case only tmStateFontDesc seems to be updated.

I'm quite happy how it went, tell me what you think about it.

@cdepillabout
Copy link
Owner

@jecaro This is really great. Thanks for taking the time to do this!

It'll probably take me a week or so to really review this, sorry for the delay.

@jecaro
Copy link
Contributor Author

jecaro commented Oct 15, 2019

You're welcome @cdepillabout and no problem for the delay.

@cdepillabout
Copy link
Owner

@jecaro I added some small commits with minor cleanup.

2dbe69d just removes whitespace on the ends of lines. ab916db changes the format of some of the code to be more consistent with the surrounding code.

c0b6bab simplifies a few functions. Could you review this commit? Once you sign off on this, it should be good to merge in.

@cdepillabout
Copy link
Owner

cdepillabout commented Oct 17, 2019

Let me try to answer the one question you had as well.

I was confused to see the settings related to the current font in tmStateFontDesc and in tmStateConfig/options/fontConfig. It is not clear to me why it needs to be that way.

The difference here is that most of the things in the TMState data type are meant to be GTK types, or other values created at runtime:

data TMState' = TMState
{ tmStateApp :: !Application
, tmStateAppWin :: !ApplicationWindow
, tmStateNotebook :: !TMNotebook
, tmStateFontDesc :: !FontDescription
, tmStateConfig :: !TMConfig
}

FontDescription is a good example of this :

http://hackage.haskell.org/package/gi-pango-1.0.22/docs/GI-Pango-Structs-FontDescription.html

However, the stuff in ConfigOptions are meant to be simple data types, easily serializable:

data ConfigOptions = ConfigOptions
{ fontConfig :: !FontConfig
-- ^ Specific options for fonts.
, showScrollbar :: !ShowScrollbar
-- ^ When to show the scroll bar.
, scrollbackLen :: !Integer
-- ^ The number of lines to keep in the scroll back history for each terminal.
, confirmExit :: !Bool
-- ^ Whether or not to ask you for confirmation when closing individual
-- terminals or Termonad itself. It is generally safer to keep this as
-- 'True'.
, wordCharExceptions :: !Text
-- ^ When double-clicking on text in the terminal with the mouse, Termonad
-- will use this value to determine what to highlight. The individual
-- characters in this list will be counted as part of a word.
--
-- For instance if 'wordCharExceptions' is @""@, then when you double-click
-- on the text @http://@, only the @http@ portion will be highlighted. If
-- 'wordCharExceptions' is @":"@, then the @http:@ portion will be
-- highlighted.
, showMenu :: !Bool
-- ^ Whether or not to show the @File@ @Edit@ etc menu.
, showTabBar :: !ShowTabBar
-- ^ When to show the tab bar.
, cursorBlinkMode :: !CursorBlinkMode
-- ^ How to handle cursor blink.
} deriving (Eq, Show)

I made this split so that ConfigOptions could be easily serialized to disk when this preferences dialog was written (maybe in a simple YAML format or something?). Then a user could potentially use and configure Termonad without having to know Haskell.

At some point I'd like to send a PR actually doing this. Or if you're interested, you could have a go at this.

@jecaro
Copy link
Contributor Author

jecaro commented Oct 17, 2019

Ok. It makes sense. Thanks for the clarification.

I think a plain config file is a good idea as well. Before I learned Haskell, I've tried a few times to use xmonad, but the config file in Haskell was a huge pain for me. I had no clue about what was trying to tell me the compiler. So yes, using a plain file would ease the adoption of termonad for the non-haskeller.

This is the kind of work I could do. I was thinking about working on #12 now. So maybe after it ? I'll see how it goes.

@cdepillabout
Copy link
Owner

This is the kind of work I could do. I was thinking about working on #12 now. So maybe after it ? I'll see how it goes.

That would be a really big help! Thanks!

I created an issue about loading and saving preferences to a file: #133

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.

2 participants