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

Adaptation to various platforms #12

Open
christophe-gouel opened this issue Aug 1, 2024 · 13 comments
Open

Adaptation to various platforms #12

christophe-gouel opened this issue Aug 1, 2024 · 13 comments

Comments

@christophe-gouel
Copy link
Collaborator

Hi @ShiroTakeda,

Thanks for giving me the right to push to the repositories.

I will soon leave on vacation, so I won't do anything else before September. In September, I would like to make some small changes for gams-mode to be easier to work with out of the box and to work better on various platforms. I have just moved my desktop computer to Linux, and I was surprised that the extraction from the gams library failed on this platform. More generally, it is quite a pain to have to maintain different emacs configs for different platforms, where GAMS will have different locations, and I am sure we can ease this.

I have the following ideas to simplify things:

  • Adjust gams-process-command-name default to (executable-find "gams"): this provides the complete path to the executable as long as it is in the PATH (on my Windows laptop, I get "c:/GAMS/47/gams.exe"; on my Linux desktop, I get "/usr/bin/gams"))
  • Then use (find-name-directory gams-process-command-name) to fill gams-system-directory default
  • Adjust gams-gamslib-command accordingly

With these changes in place, people would not see any difference if they had customized the variables. But this should streamline the gams-mode config since there won't be any path to set if the GAMS folder is in the PATH.

What do you think?

@ShiroTakeda
Copy link
Owner

Since I primarily use Windows systems, I have very little understanding of the situation with GAMS on other platforms or the issues that might arise with GAMS mode.

I think your suggestion is a very good idea.

However, the following command

(find-name-directory gams-process-command-name)

will actually be the following command:

(file-name-directory gams-process-command-name)


Changing the topic a bit, one of the issues with GAMS mode is that the program file (gams-mode.el) is too large.

Emacs Lisp packages of a certain size are usually created by splitting the program into multiple files, typically divided by functionality. This approach makes the program easier to understand and manage.

However, since I have developed almost all parts of GAMS mode by myself, there hasn’t been much need to enhance readability for others, leading to the inclusion of everything in a single file.

While there may be some advantages to including everything in one file, the current gams-mode.el exceeds 17,000 lines, making it quite difficult to understand. Even I find it challenging to keep track of where I wrote what.

Ideally, GAMS mode should also be divided into files by functionality. However, it is not easy to do this effectively, so that task has remained untouched.

If there is anything you don't understand in the program, please feel free to ask.

@christophe-gouel
Copy link
Collaborator Author

Changing the topic a bit, one of the issues with GAMS mode is that the program file (gams-mode.el) is too large.

Emacs Lisp packages of a certain size are usually created by splitting the program into multiple files, typically divided by functionality. This approach makes the program easier to understand and manage.

However, since I have developed almost all parts of GAMS mode by myself, there hasn’t been much need to enhance readability for others, leading to the inclusion of everything in a single file.

While there may be some advantages to including everything in one file, the current gams-mode.el exceeds 17,000 lines, making it quite difficult to understand. Even I find it challenging to keep track of where I wrote what.

Ideally, GAMS mode should also be divided into files by functionality. However, it is not easy to do this effectively, so that task has remained untouched.

Yes, gams-mode.el is too large, but I don't think anything should be done about this. GAMS mode is stable at this point. I am just contributing minor enhancements, which I am able to do even with this large file. I don't think it is worth anyone's time to break it down into smaller files.

The only thing I would suggest is to change the comments to follow the outline-minor-mode standards. This would make code navigation much easier in the file (org-like). In lisp-mode, sections starts by ;;;, subsections by ;;;;, ;;;;;, ...

So a code block such as

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;;
;;;     Define variables.
;;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

could be changed in

;;;; Define variables

and it would be a subsection inside the ;;; Code: section.

Alternatively, it could be

;;; Define variables

if one wants it to be a section (this is the convention in Magit for example).

@christophe-gouel christophe-gouel mentioned this issue Oct 1, 2024
@christophe-gouel
Copy link
Collaborator Author

Hi @ShiroTakeda,

I have done the adjustments required for GAMS mode to behave better on all platforms. The changes are in the develop branch. They should not be merged in master right now. Some testing is required first, which I will do in the following weeks, but I encourage you to try it too.

I have also to edit the README to update the configuration instructions to the changes. I will also do this soon.

There are a lot of changes in the file, but those are mostly small fixes to compilation warnings/notes. The substantial changes are limited and described in the CHANGELOG.md. I have taken the liberty to update the version number to 7.0 since some of the changes can break backward compatibility in minor ways.

Best,
Christophe

@christophe-gouel
Copy link
Collaborator Author

Hi @ShiroTakeda,

I am thinking about adding from faces for common mathematical operators (-, +, *, **, /). This would ease a lot reading equations, especially when people do not leave spaces around operators. Such faces are for example use in the mode for R. Here are 2 examples of the same equation in GAMS and R modes:

GAMS mode:
image

R mode:
image

I find the R mode version more readable.

Would you have an objection to this change?

@ShiroTakeda
Copy link
Owner

I have no particular objections, so please go ahead and add it. However, symbols such as * are also used for comments, so they might conflict with other functions of the program. For now, please try adding it as a test.

@christophe-gouel
Copy link
Collaborator Author

Great, I have just pushed the changes in the develop branch. There is problem of conflicts with comments or other things.

@ShiroTakeda
Copy link
Owner

By the way, if the changes you made to the develop branch don't cause any particular problems, you can merge them into the master branch. Alternatively, I can merge them for you.

I don't think there are any problems with the changes you made in November.

@christophe-gouel
Copy link
Collaborator Author

Thanks for validating the changes.

I would like to add a bit of documentation and check everything before merging. I would do it sometimes in January.

@christophe-gouel
Copy link
Collaborator Author

I would like to know what is the interest of the function gams-recenter?

I understand that it recenters the font-locking and the point, but is this really necessary as of now? With recent emacs versions and recent computers, font locking is no longer an issue in terms of computational power. And by binding it to C-l, it overwrites the recenter-top-bottom which makes more than recentering (it also moves the point to the top and the bottom).

Do you think it is still necessary?

@ShiroTakeda
Copy link
Owner

If the value of the variable jit-lock-chunk-size is not set sufficiently high, there are cases where the font-lock coloring does not function properly. For example, in the file named gams-sample.gms, which is distributed along with gams-mode.el, some parts may not be colored correctly. To address this issue, I added a function called gams-recenter, which performs coloring while recentering, and I bound it to C-l.

However, the issue of improper coloring can be resolved by setting a sufficiently large value for jit-lock-chunk-size. Furthermore, the file gams-sample.gms may be a special file with a very long comment section enclosed by $ontext-$offtext, so the problem of coloring not working properly may rarely occur.

Therefore, I believe there is no particular problem in discontinuing the binding of gams-recenter to C-l (i.e., leaving C-l as recenter-top-bottom). Additionally, it might be appropriate to remove the code related to gams-recenter.

In gams-mode.el, there might be other features that are less meaningful or not used at all. For example, I do not use the GAMS-LXI mode myself, so I think it does not serve much purpose. It might be a good idea to revise or remove such code.

@ShiroTakeda
Copy link
Owner

Previously, in the BUGS_PROBLEMS.md I explained the cases where font-lock does not work well (and how to deal with them).

https://github.com/ShiroTakeda/gams-mode/blob/a18b05861f95f6c232bd2d3ebf247956890911c8/BUGS_PROBLEMS.md

But now it is not in BUGS_PROBLEMS.md because I removed it in the following commit: 51ab6b5

@christophe-gouel
Copy link
Collaborator Author

OK I get it.

Thanks for your explanations.

I have looked as my config and my jit-log-chunk-size is at 50000, so it is not a surprise if I am not seeing many problems these days. However, if I revert it back to its default, I see the problems appear in your sample file.

I think that many big models have large comment blocks, so this is likely to be a common issue.

At this point, I think it better to leave the command as it is. I will just deactivate it in my config since I have no use for it.

However, I will add to the README file the recommentation about increasing jit-log-chunk-size. These days, with the power of modern computers, there is very limited cost of setting it at a high value. It should be at least at a value of 5000 to cover 2 large screen sizes with a lot of comments.

@ShiroTakeda
Copy link
Owner

I think your suggested revision is fine.

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

No branches or pull requests

2 participants