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

[WIP] RFC 3: languages #28

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[WIP] RFC 3: languages #28

wants to merge 6 commits into from

Conversation

shahrukh330
Copy link
Contributor

@shahrukh330 shahrukh330 commented May 1, 2021

@shahrukh330 shahrukh330 requested a review from kalbasit May 1, 2021 16:42
rfcs/0003-languages.md Outdated Show resolved Hide resolved
rfcs/0003-languages.md Outdated Show resolved Hide resolved
rfcs/0003-languages.md Outdated Show resolved Hide resolved
rfcs/0003-languages.md Outdated Show resolved Hide resolved
rfcs/0003-languages.md Outdated Show resolved Hide resolved
rfcs/0003-languages.md Outdated Show resolved Hide resolved
`Array` of `str`, with apply function which iterate over the array and find
appropriate language or tool.

### Per-editor
Copy link
Member

Choose a reason for hiding this comment

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

That would probably not be editor specific. When you enable go, for example, you might want to have the Go compiler in your environment. It might also configure your shell prompt to change depending on whether you are in a Python virtual environment. It would probably be per-program or per-module.
Note: this is not the only place in the RFC where that nomenclature should be changed.

### Per-editor

Each editor that supports a language must provide a
`soxin.programs.<editor-name>.language` and `soxin.programs.<editor-name>.tool`
Copy link
Member

Choose a reason for hiding this comment

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

So shouldn't it be soxin.programs.<editor-name>.programming.language?

Comment on lines 140 to 141
# Examples and Interactions
[examples-and-interactions]: #examples-and-interactions
Copy link
Member

Choose a reason for hiding this comment

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

This is duplicated.

Comment on lines +144 to +145
# Drawbacks
[drawbacks]: #drawbacks
Copy link
Member

Choose a reason for hiding this comment

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

One drawback I might add, is that we might not make the difference between soxin.programs and soxin.tools. What would it be and what are the criteria for putting a module under programs rather than tools?

Copy link
Member

Choose a reason for hiding this comment

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

Can we add a definition section at the top of the RFC to explain this?

My 2cents: go is a language but gorename is not, it's a tool. Go like any other language may also enable one or more tools.

rfcs/0003-languages.md Outdated Show resolved Hide resolved

Each program that supports a language must provide a
`soxin.programs.<program-name>.programming.language` and
`soxin.programs.<editor-name>.programming.tool` option to allow the user to
Copy link
Member

Choose a reason for hiding this comment

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

I think we should avoid specifying the type of the module. So instead of saying each program we should say each module.


let
cfg = config.soxin.programs.neovim;
goSupportRc = cfg.programming.languages.go ? "";
Copy link
Member

Choose a reason for hiding this comment

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

The returned type of this is an attrset, not a string; Shouldn't it be .go ? {}?

${gitSupportRc}
'';

plugins = [ /* Some plugins */ ] ++ goSupportPlugins ++ gitSupportPlugins;
Copy link
Member

Choose a reason for hiding this comment

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

Thinking about this, I don't think it should say go or git, instead, it should add the RC and plugins of all enabled language and tools in soxin's settings.

Comment on lines +144 to +145
# Drawbacks
[drawbacks]: #drawbacks
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a definition section at the top of the RFC to explain this?

My 2cents: go is a language but gorename is not, it's a tool. Go like any other language may also enable one or more tools.

* 'main' of github.com:SoxinOS/soxin: (21 commits)
  flake: follow nixpkgs on master (#44)
  treewide: move the project to using flake-utils-plus (#41)
  pkgs: deprecate stdenv.lib (#42)
  programs.starship: init module (#40)
  programs.less: init module (#40)
  programs.keybase: init module (#40)
  programs.git: init module (#40)
  programs.autorandr: init module (#40)
  modules: sort the list of modules per type (#40)
  lib: homeManagerConfiguration support for direct home-manager support (#37)
  themes.gruvbox-dark: add support for i3 and polybar (#36)
  programs.termite: initial import with theme support (#35)
  programs.tmux: initial import with theme support (#34)
  programs.zsh: initial import with theme support (#33)
  programs.neovim: initial import with theme support (#31)
  [lib] nixosSystem: allow setting global/nixos/hm modules and update to latest HM (#32)
  treewide: format all files
  programs.rofi: add support for swapping i3 workspaces (#30)
  programs.rofi: update to support master branch of HM (#30)
  programs.rofi: refactor: compute the modi in the apply function (#30)
  ...
Co-authored-by: Wael Nasreddine <wael.nasreddine@gmail.com>
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

3 participants